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

Looser type annotations for wrap_file() and FileWrapper #2130

Closed
srittau opened this issue May 17, 2021 · 8 comments
Closed

Looser type annotations for wrap_file() and FileWrapper #2130

srittau opened this issue May 17, 2021 · 8 comments
Labels
Milestone

Comments

@srittau
Copy link
Contributor

srittau commented May 17, 2021

werkzeug.wsgi.wrap_file() and FileWrapper require the file argument to have type BinaryIO. Generally, the use of IO classes is discouraged, especially as arguments, since they are normally much too strict and incompatible with each other. In my particular use case, I got an error message 'Argument 2 to "wrap_file" has incompatible type "IO[bytes]"; expected "BinaryIO"'.

As FileWrapper only requires the read() method to be present (although it uses more methods if they exist), the old typeshed stubs annotated the file arguments with SupportsRead[bytes], which is available in _typeshed during type checking:

if t.TYPE_CHECKING:
    from _typeshed import SupportsRead

Alternatively a similar protocol can be defined like this:

_T_co = t.TypeVar("_T_co", covariant=True)
class SupportsRead(t.Protocol[_T_co]):
    def read(self, __length: int = ...) -> _T_co: ...
@davidism
Copy link
Member

Is there a reason this isn't available in typing_extensions? I'm worried about importing _typeshed, which seems like a private namespace that might not be available to type checkers besides mypy.

@srittau
Copy link
Contributor Author

srittau commented May 17, 2021

_typeshed is available on all type checkers, since it's "part" of the stdlib, as defined by typeshed. In fact, werkzeug already uses it indirectly: WSGIEnvironment is imported from the deprecated wsgiref.types namespace, which is just a star import from _typeshed.wsgi. There isn't any real API guarantee about _typeshed, but SupportsRead is not going anywhere soon. It's not the greatest place and (my) ultimate plan is to add some of the types to the standard library, but it's the best place we have at the moment. Although as you point out, a good intermediate plan could be to use typing_extensions for some proven types.

That said, if you are uncomfortable using _typeshed (which I can totally understand), maybe you could consider adding a werkzeug.types module, where you can just copy & paste types used by werkzeug.

@davidism
Copy link
Member

That's fine then, is there documentation about the _typeshed namespace and what it provides? Had no idea about wsgiref.types, that would be good to refactor to the _typeshed.wsgi name.

@srittau
Copy link
Contributor Author

srittau commented May 17, 2021

We don't have documentation at the moment, except for the comments in the file itself, but we should probably add some. I will bring it up!

@davidism davidism added this to the 2.0.1 milestone May 17, 2021
@davidism
Copy link
Member

Using SupportsRead[bytes] brings up a bunch of errors since hasattr isn't recognized by mypy to allow other methods, so you get things like "SupportsRead[bytes] has no attribute close". On the other hand, IO[bytes] works, and seems to also satisfy the issue you're having.

@davidism davidism removed this from the 2.0.1 milestone May 17, 2021
@davidism
Copy link
Member

There's lots of other places that use BinaryIO and TextIO, so I'd like to understand what can be changed with those as well. This doesn't seem immediately urgent, and I want to get 2.0.1 out, so this can go in 2.0.2.

@srittau
Copy link
Contributor Author

srittau commented May 19, 2021

Good point about hasattr(). I totally missed that. There is some previous discussions about "optional" protocol fields in python/typing#601. I agree that using IO[bytes] for now is the most pragmatic solution.

BinaryIO and TextIO have a few more methods defined over IO, but usually nothing relevant when expecting them as an argument. They can be useful when returning them, because they have __enter__() methods:

with my_binary_io_returning_open():
    ...

@MaicoTimmerman
Copy link
Contributor

I'm running in the same typing issue with FileStorage, I've extracted this snippet from our project:

from zipfile import ZipFile
from werkzeug.datastructures import FileStorage


with ZipFile(FileStorage()) as zf:
    pass
test.py:5: error: Argument 1 to "ZipFile" has incompatible type "FileStorage"; expected "Union[Union[str, _PathLike[str]], IO[bytes]]"
Found 1 error in 1 file (checked 1 source file)

@davidism davidism closed this as completed Aug 6, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants