Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_WriteTextMode etc. results in very unhelpful typing errors #6719

Closed
rhpvorderman opened this issue Dec 28, 2021 · 4 comments
Closed

_WriteTextMode etc. results in very unhelpful typing errors #6719

rhpvorderman opened this issue Dec 28, 2021 · 4 comments

Comments

@rhpvorderman
Copy link
Contributor

Example

def my_custom_open(filename, mode: str = "rt"):
    # do stuff here
    bz2.open(filename, mode)

This results in suggestions like this:

def open(filename: _ReadableFileobj, mode: Union[Literal[''], Literal['r'], Literal['rb']] = ..., compresslevel: int = ..., encoding: None = ..., errors: None = ..., newline: None = ...) -> BZ2File
 def open(filename: _ReadableFileobj, mode: Literal['rt'], compresslevel: int = ..., encoding: Optional[str] = ..., errors: Optional[str] = ..., newline: Optional[str] = ...) -> TextIO
 def open(filename: _WritableFileobj, mode: Union[Literal['w'], Literal['wb'], Literal['x'], Literal['xb'], Literal['a'], Literal['ab']], compresslevel: int = ..., encoding: None = ..., errors: None = ..., newline: None = ...) -> BZ2File
 def open(filename: _WritableFileobj, mode: Union[Literal['wt'], Literal['xt'], Literal['at']], compresslevel: int = ..., encoding: Optional[str] = ..., errors: Optional[str] = ..., newline: Optional[str] = ...) -> TextIO
 def open(filename: Union[str, bytes, PathLike[str], PathLike[bytes]], mode: Union[Union[Literal[''], Literal['r'], Literal['rb']], Union[Literal['w'], Literal['wb'], Literal['x'], Literal['xb'], Literal['a'], Literal['ab']]] = ..., compresslevel: int = ..., encoding: None = ..., errors: None = ..., newline: None = ...) -> BZ2File
def open(filename: Union[str, bytes, PathLike[str], PathLike[bytes]], mode: Union[Literal['rt'], Union[Literal['wt'], Literal['xt'], Literal['at']]], compresslevel: int = ..., encoding: Optional[str] = ..., errors: Optional[str] = ..., newline: Optional[str] = ...) -> TextIO

This is ridiculous.

  • The typing system does not run at runtime. So it can't check whether my string is in the supported modes.
  • bz2.open does check whether the mode is correct. No need for the typing system to do this.
  • bz2.open does accept any string for a mode. This will not raise a TypeError but an appropriate ValueError if the value is incorrect.

Mode is a str nothing more, nothing less. It is good that the typesystem checks that I do not accidentally pass an int or a float or a List[str] but this is taking it way too far. It is extremely unhelpful, it leads to an enormous amount of bloat in the code. Is it really required for me to write every accepted Literal mode under the sun? This is creating the m times n problem all over again.

I guess this can be solved by adding a simple def open(filename: Union[str, bytes, PathLike[str], PathLike[bytes]], mode: str, compresslevel: int = ..., encoding: Optional[str] = ..., errors: Optional[str] = ..., newline: Optional[str] = ...) -> ... definition to typeshed, so I will do that in a PR.

@srittau
Copy link
Collaborator

srittau commented Dec 28, 2021

Finding these kinds of problems is exactly what static type checking is intended for. bz2.open() does not accept arbitrary strings. Whether it raises TypeError or ValueError does not matter. Additionally, the return type depends on the chosen mode, so it can't be ignored.

We won't change this in typeshed. In your custom function you can either use the mode aliases from bz2, use your own set of compatible literals, or use cast() or # type: ignore if you want to keep the imprecise str annotation.

@srittau srittau closed this as completed Dec 28, 2021
@Akuli
Copy link
Collaborator

Akuli commented Dec 28, 2021

You might like my @copy_type decorator from python/typing#769 .

@rhpvorderman
Copy link
Contributor Author

Thank you for your quick reply!

Finding these kinds of problems is exactly what static type checking is intended for. .... Whether it raises TypeError or ValueError does not matter.

This is the core of the disagreement. This repository is called typeshed not valueshed and that shaped my expectations.

Additionally, the return type depends on the chosen mode, so it can't be ignored.

But there is the typing.IO type for these use cases, if I have understood it correctly?

Anyway, I am sorry for being so blunt in the top post. I could have phrased it in a much less abrasive manner. What it comes down to fundamentally is that I feel that I more generic use with mode: str and -> typing.IO is more helpful and easier to work with.

We won't change this in typeshed

Thank you for providing a clear motivation for why this is the case and also providing possible alternatives. (Almost) Happy New Year!

@Akuli
Copy link
Collaborator

Akuli commented Dec 28, 2021

typing.IO seems useful until you have a function that calls a method .readlines() on a file object, and another function that returns an object that only supports .read(). Or a function that usually calls .write(), but might also call .flush() depending on argument (print does this), and some file objects that can't be flushed.

Typeshed now avoids typing.IO, and instead uses specific callback protocols that say e.g. "accepts any object with .readlines()", and concrete return types e.g. "returns a TextIOWrapper".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants