Skip to content

Deprecate _typeshed.Supports{Read,Write} #14149

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented May 26, 2025

Replace with typing_extensions.{Reader,Writer} in stdlib

Replace with `typing_extensions.{Reader,Writer}` in stdlib
@srittau
Copy link
Collaborator Author

srittau commented May 26, 2025

Draft until typing_extensions 4.14.0 final is released. I'll give the third-party stubs a week or two after the release.

This comment has been minimized.

@srittau
Copy link
Collaborator Author

srittau commented May 26, 2025

This is a bit more primer output than expected (due to Writer.write() requiring to return int, while SupportsWrite.write() had just object as return type). Looking at it more closely:

  • sphinx no 1: SafeEncodingWriter.write() and LastMessagesWriter.write() currently return None, but changing it would be trivial (and a good idea for compatibility). sphinx.util.logging: Make write() methods return int sphinx-doc/sphinx#13595
  • sphinx no 2: One annotation needs to be changed from SupportsWrite to Writer.
  • pandas is actually a potential problem with our codecs stubs. Will investigate further. codecs.StreamWriter.write() returns None cpython#134706
    • Well, since the CPython developers are unwilling to fix their bug, pandas will have to live with a # type: ignore in their code or just switch from the non-conforming codecs API.
  • cwltool: Also needs one annotation changed from SupportsWrite to Writer.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Jun 2, 2025

Diff from mypy_primer, showing the effect of this PR on open source code:

django-stubs (https://github.com/typeddjango/django-stubs)
+ django-stubs/core/management/utils.pyi:4: error: class _typeshed.SupportsWrite is deprecated: Use typing_extensions.Writer instead. Will be removed in December 2025 or later.  [deprecated]
+ django-stubs/core/serializers/json.pyi:8: error: class _typeshed.SupportsRead is deprecated: Use typing_extensions.Reader instead. Will be removed in December 2025 or later.  [deprecated]

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/util/logging.py: note: In class "WarningStreamHandler":
+ sphinx/util/logging.py:204:50: error: Type argument "SafeEncodingWriter" of "StreamHandler" must be a subtype of "Writer[str]"  [type-var]
+ sphinx/util/logging.py: note: In class "NewLineStreamHandler":
+ sphinx/util/logging.py:210:50: error: Type argument "SafeEncodingWriter" of "StreamHandler" must be a subtype of "Writer[str]"  [type-var]
+ sphinx/util/logging.py: note: In function "setup":
+ sphinx/util/logging.py:636:26: error: No overload variant of "StreamHandler" matches argument type "LastMessagesWriter"  [call-overload]
+ sphinx/util/logging.py:636:26: note: Possible overload variants:
+ sphinx/util/logging.py:636:26: note:     def [_StreamT: Writer[str]] __init__(self, stream: None = ...) -> StreamHandler[TextIO]
+ sphinx/util/logging.py:636:26: note:     def [_StreamT: Writer[str]] __init__(self, stream: _StreamT) -> StreamHandler[_StreamT]
+ sphinx/_cli/util/errors.py: note: In function "handle_exception":
+ sphinx/_cli/util/errors.py:183:9: error: No overload variant of "print" matches argument types "tuple[str, ...]", "SupportsWrite"  [call-overload]
+ sphinx/_cli/util/errors.py:183:9: note: Possible overload variants:
+ sphinx/_cli/util/errors.py:183:9: note:     def print(*values: object, sep: str | None = ..., end: str | None = ..., file: Writer[str] | None = ..., flush: Literal[False] = ...) -> None
+ sphinx/_cli/util/errors.py:183:9: note:     def print(*values: object, sep: str | None = ..., end: str | None = ..., file: _SupportsWriteAndFlush[str] | None = ..., flush: bool) -> None

pandas (https://github.com/pandas-dev/pandas)
+ pandas/util/_print_versions.py:147: error: Argument 2 to "dump" has incompatible type "StreamReaderWriter"; expected "Writer[str]"  [arg-type]
+ pandas/util/_print_versions.py:147: note: Following member(s) of "StreamReaderWriter" have conflicts:
+ pandas/util/_print_versions.py:147: note:     Expected:
+ pandas/util/_print_versions.py:147: note:         def write(self, str, /) -> int
+ pandas/util/_print_versions.py:147: note:     Got:
+ pandas/util/_print_versions.py:147: note:         def write(self, data: str) -> None

cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/executors.py: note: In member "run_jobs" of class "SingleJobExecutor":
+ cwltool/executors.py:243:34: error: Argument "file" to "print" has incompatible type "SupportsWrite[str] | None"; expected "Writer[str] | None"  [arg-type]

@AA-Turner
Copy link
Member

AA-Turner commented Jun 3, 2025

sphinx-doc/sphinx#13595 points to this issue for background, but I'm still missing context. Why is this change required? It breaks working code.

@AlexWaygood
Copy link
Member

The discussions in python/cpython#134706 and sphinx-doc/sphinx#13595 make me think that we should update the new Writer protocol in CPython and typing-extensions so that Writer.write() returns object rather than int. I don't recall there ever being any issues raised at typeshed about the return type of _typeshed.Writer.write(), and there doesn't appear to be consensus that write() methods should always return an integer.

@srittau
Copy link
Collaborator Author

srittau commented Jun 3, 2025

typing.IO.write() as well as basically all classes in the stdlib (especially the base classes in io) return an int.

@AA-Turner
Copy link
Member

AA-Turner commented Jun 3, 2025

Given how old the write() protocol is, I think to do something useful with the return value one either needs to know more about the semantics/type of the writer than that it has a .write() method, or check the return type and use it if it's an int, or etc. I agree with Alex that object would be a better choice. I would hazard that int | None might work, but I can't remember the current view on union types as returned types.

A

@srittau
Copy link
Collaborator Author

srittau commented Jun 3, 2025

I'm unwilling to change the protocol. As the primer hits show, there is no problem with returning int, unless you use codecs directly, which isn't recommended anyway. Returning the number of items written has been standard since C days, and there is basically no reason to deviate from that. The arguments brought forth to change this standard are shallow and short sighted.

@srittau
Copy link
Collaborator Author

srittau commented Jun 3, 2025

And anyway, this discussion is off topic for this PR. _typeshed.SupportsWrite should go, no matter whether io.Writer is changed or not.

@AlexWaygood
Copy link
Member

And anyway, this discussion is off topic for this PR. _typeshed.SupportsWrite should go, no matter whether io.Writer is changed or not.

I think the discussion is pretty on-topic. Looking at the proposed change here purely in terms of the types, rather than which protocol is in the standard library for this PR: this PR switches lots of annotations so that various functions now require an object with a write() method that returns some subtype of int rather than an object with a write() method that returns some subtype of object. What are the specific advantages to requiring that write() returns some subtype of int for ArgumentParser.print_usage(), for example, where the return type is never used?

I think we should aim to give users of type checkers the best experience possible, so I'm not sure why we should accept any mypy_primer fallout here unless there's a good argument that there were type-safety issues or false positives there beforehand.

@srittau
Copy link
Collaborator Author

srittau commented Jun 3, 2025

I think we should aim to give users of type checkers the best experience possible, so I'm not sure why we should accept any mypy_primer fallout here unless there's a good argument that there were type-safety issues or false positives there beforehand.

As pointed out above the fallout is minimal. Projects need to switch away from SupportsWrite anyway, leaving two easy to fix hits.

@AA-Turner
Copy link
Member

To take the Sphinx case as an example, the classes were added a decade ago and are mostly unchanged since then. I am unconvinved by the 'minimal fallout' argument, we should be convinced that a change is correct, rather than that doesn't impact many people. The write() (small p) protocol is defined nowhere in the documentation, and so I think an attempt to codify it in the type system should be descriptive rather than prescriptive. Generally when an arbitrary write() method is spoken of, the parameters are discussed, I haven't yet found a case where a prose description of the protocol requires any return type.

A few examples I quickly found of the documentation not following the -> int contract:

A

@srittau
Copy link
Collaborator Author

srittau commented Jun 3, 2025

I've answered these arguments in the CPython PR. As I said before, I believe (sync) write() methods are supposed to return the number of items written, a convention that goes back at least 40 years. Diverging behavior is a bug for no good reason. The current protocol has been included in at least two beta releases by now, changing it to accomodate APIs that don't conform to this ancient convention is not worth the change (and regression in functionality).

@AlexWaygood
Copy link
Member

The current protocol has been included in at least two beta releases by now

It should be fairly easy to change the return type at any point before the release candidates come out. We've made much bigger changes to new features during beta periods in the past over at CPython; the purpose of beta periods is meant to be for users to play with new features and report sharp edges in them before the release-candidate phase.

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

Successfully merging this pull request may close these issues.

3 participants