Fixes Pyright type errors for src/flask/app.py
#5544
Conversation
brendon-codes
force-pushed
the
pyright-types-fixes-app
branch
from
54abc56 to
adbc23d
Compare
| # a 3-tuple is unpacked directly | ||
| if len_rv == 3: | ||
| rv, status, headers = rv # type: ignore[misc] | ||
| if len(rv) == 3: |
There was a problem hiding this comment.
I don't understand why this code was refactored. Can you address the issue with more minimal code changes?
There was a problem hiding this comment.
Unfortunately the pyright inferrence doesn't seem to be smart enough to narrow down the type inside the conditional here when using the intermediate len_rv assignment..
There was a problem hiding this comment.
Please report that issue to Pyright and link it here so we can track when that's fixed. I don't like the idea of refactoring this in this way just to satisfy pyright. This is code that runs for every single response and so adding extra calls is not free.
There was a problem hiding this comment.
Ok, I can do that. If it seems like that either won't get fixed anytime soon, should we use a cast() here to help it along rather than # ignore so we at least get a little closer to type safety?
There was a problem hiding this comment.
Prefer ignore, since a comment has no runtime cost.
There was a problem hiding this comment.
Ok, that sounds good. Thanks for being patient on this.
| @@ -322,7 +323,7 @@ def send_static_file(self, filename: str) -> Response: | |||
|
|
|||
| def open_resource( | |||
| self, resource: str, mode: str = "rb", encoding: str | None = None | |||
| ) -> t.IO[t.AnyStr]: | |||
| ) -> t.IO[t.AnyStr] | BufferedReader: | |||
There was a problem hiding this comment.
It seems like this should be reported to Pyright. Why doesn't t.IO satisfy buffered reader? Why is this a union, if t.IO isn't correct; shouldn't it be only BufferedReader?
There was a problem hiding this comment.
Here is the builtin types for open() (See: https://github.com/python/typeshed/blob/main/stdlib/builtins.pyi#L1540). t.IO[Any] is the fallback if no mode is specified. So it doesn't always crossover with BufferedReader:
@overload
def open(
file: FileDescriptorOrPath,
mode: OpenTextMode = "r",
buffering: int = -1,
encoding: str | None = None,
errors: str | None = None,
newline: str | None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> TextIOWrapper: ...
# Unbuffered binary mode: returns a FileIO
@overload
def open(
file: FileDescriptorOrPath,
mode: OpenBinaryMode,
buffering: Literal[0],
encoding: None = None,
errors: None = None,
newline: None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> FileIO: ...
# Buffering is on: return BufferedRandom, BufferedReader, or BufferedWriter
@overload
def open(
file: FileDescriptorOrPath,
mode: OpenBinaryModeUpdating,
buffering: Literal[-1, 1] = -1,
encoding: None = None,
errors: None = None,
newline: None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> BufferedRandom: ...
@overload
def open(
file: FileDescriptorOrPath,
mode: OpenBinaryModeWriting,
buffering: Literal[-1, 1] = -1,
encoding: None = None,
errors: None = None,
newline: None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> BufferedWriter: ...
@overload
def open(
file: FileDescriptorOrPath,
mode: OpenBinaryModeReading,
buffering: Literal[-1, 1] = -1,
encoding: None = None,
errors: None = None,
newline: None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> BufferedReader: ...
# Buffering cannot be determined: fall back to BinaryIO
@overload
def open(
file: FileDescriptorOrPath,
mode: OpenBinaryMode,
buffering: int = -1,
encoding: None = None,
errors: None = None,
newline: None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> BinaryIO: ...
# Fallback if mode is not specified
@overload
def open(
file: FileDescriptorOrPath,
mode: str,
buffering: int = -1,
encoding: str | None = None,
errors: str | None = None,
newline: str | None = None,
closefd: bool = True,
opener: _Opener | None = None,
) -> IO[Any]: ...There was a problem hiding this comment.
OK, so typeshed says that we're returning BufferedReader. But is that a guarantee we want to make vs returning the more general IO? I need more information to understand this change, "it passes pyright" isn't enough because it also changes what we report our API to be.
|
Please make one PR with the fixes you're identifying. Opening a new issue and PR per file isn't helpful for this. |
Should I also make one single issue? |
Fixes Pyright type errors for
src/flask/app.py.fixes #5543