typing: accept buffers in typing.IO.write #9084
Conversation
|
The mypy failures are a bug, python/mypy#14002. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This looks quite disruptive to me and I'm not sure whether it's worth it, considering the slightly legacy nature of |
|
Worth noting that most of the mypy-primer fallout is due to a mypy bug (python/mypy#14002). |
|
This seems useful, but given that I think I have a mypy fix, let's wait until it's released: python/mypy#14017 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
stdlib/codecs.pyi
Outdated
| @@ -239,7 +239,7 @@ class StreamReaderWriter(TextIO): | |||
| def __next__(self) -> str: ... | |||
| def __iter__(self: Self) -> Self: ... | |||
| def write(self, data: str) -> None: ... # type: ignore[override] | |||
| def writelines(self, list: Iterable[str]) -> None: ... | |||
| def writelines(self, list: Iterable[str]) -> None: ... # type: ignore[override] # https://github.com/python/mypy/issues/14002 | |||
There was a problem hiding this comment.
We probably don't need this type: ignore anymore (as well as several others), with the latest mypy release.
There was a problem hiding this comment.
Cleaned up all comments referencing that issue.
stdlib/http/client.pyi
Outdated
| @@ -101,7 +101,7 @@ class HTTPMessage(email.message.Message): | |||
|
|
|||
| def parse_headers(fp: io.BufferedIOBase, _class: Callable[[], email.message.Message] = ...) -> HTTPMessage: ... | |||
|
|
|||
| class HTTPResponse(io.BufferedIOBase, BinaryIO): | |||
| class HTTPResponse(io.BufferedIOBase, BinaryIO): # type: ignore[misc] # https://github.com/python/mypy/issues/14002 | |||
There was a problem hiding this comment.
We do still need this type: ignore, but not because of the mypy issue linked (which is now closed) -- we need it because the definitions of writelines() in the two base classes are incompatible. The same goes for all the type: ignores in io.pyi and the one in lzma.pyi.
There was a problem hiding this comment.
Updated these comments.
stdlib/tempfile.pyi
Outdated
| @overload | ||
| def write(self: _TemporaryFileWrapper[str], s: str) -> int: ... | ||
| @overload | ||
| def write(self: _TemporaryFileWrapper[bytes], s: ReadableBuffer) -> int: ... | ||
| @overload | ||
| def writelines(self: _TemporaryFileWrapper[str], lines: Iterable[str]) -> None: ... | ||
| @overload | ||
| def writelines(self: _TemporaryFileWrapper[bytes], lines: Iterable[ReadableBuffer]) -> None: ... |
There was a problem hiding this comment.
There was a problem hiding this comment.
Changed, and added some testcases for IO.write.
This comment has been minimized.
This comment has been minimized.
|
Ping @JelleZijlstra, have you had a chance to take a look at my review feedback? :) |
|
Oops, I think I assumed this was still blocked. I will take a look soon! |
This comment has been minimized.
This comment has been minimized.
|
hm, the new way to write the overloads makes subclassing much worse. Let me see if I can fix that. |
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: rich (https://github.com/Textualize/rich)
+ rich/progress.py:173: error: Definition of "writelines" in base class "IOBase" is incompatible with definition in base class "IO" [misc]
psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/copy.py:464: error: Unused "type: ignore" comment
cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/main.py:963: error: Unused "type: ignore[arg-type]" comment
|
|
Seems like we have a choice between:
I'm inclined to go with (1) because mypy's behavior is more clearly buggy than pyright's, but ideally we'd fix the mypy bug first. |
|
Now that python/mypy#14882 has been merged, I guess we have two options:
I'm easy either way :) |
Fixes #9082