FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix annotations for `builtins.reversed.__new__` by jorenham · Pull Request #11646 · python/typeshed · GitHub

Repository navigation

Fix annotations for builtins.reversed.__new__ - #11646

Merged
srittau merged 5 commits into
python:mainfrom
jorenham:re-versed
Jul 23, 2026
Merged

srittau merged 5 commits into
python:mainfrom
jorenham:re-versed

Conversation

jorenham commented Mar 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

fixes #11645


Deferred due to a mypy bug: python/mypy#15182

This comment has been minimized.

This comment has been minimized.

srittau commented Mar 23, 2024

Copy link
Copy Markdown
Collaborator

It seems that mypy has problems with the first overload, but I'm not sure why. Everything seems correct to me and pyright seems to agree. For now, I would suggest to split this PR in two, one PR for the second overload and a separate PR for the first one. This should allow us to get part of the changes into typeshed soon, and will make it easier to debug the mypy problem.

jorenham commented Mar 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@srittau I have tried many different approaches, but none of them seem to be accepted by mypy. As you said, pyright has no problem with this. I can't draw any other conclusions than this being a mypy bug, and it's easy to reproduce:

https://mypy-play.net/?mypy=latest&python=3.12&gist=0c754a7826a80f50ced671c26d891950&flags=strict

The expected behavior can be illustrated by pyright, which works as expected and doesn't require the type: ignore[misc]:
https://pyright-play.net/?strict=true&code=GYJw9gtgBALgngBwJYDsDmUkQWEMoAqiApgGoCGIAUFQVALyEkUgAUA5AewJQ1UDGAG3IBnEVAByYGAEEiCYgC4qUVVAAmxYFAD6OlMQDue1kJEAaKJTSLC3KAFoAfIWVr3UEMRgBXECisQNBUPVRDQqi8AN2JyQR14BVYpWXliVgAWACZubiA

I wasn't able to find anything on https://typing.readthedocs.io/ that would restrict the type of the return value from a (non-meta) class' __new__ method.
But for some reason, mypy blindly assumes that this is always a covariant type that is bound to the class at hand (i.e. __new__: (cls, ...) -> Self with Self somehow being covariant). Any annotations that suggest otherwise result in a type error in the misc category. But even if that error is explicitly suppressed, mypy will implicitly ignore it.

srittau added the status: deferred Issue or PR deferred until some precondition is fixed label Mar 25, 2024

srittau commented Mar 25, 2024

Copy link
Copy Markdown
Collaborator

I've marked this PR as deferred for now due to the mypy bug.

Copy link
Copy Markdown
Contributor Author

@srittau I don't quite follow.

Currently, all typecheckers will reject a reversed call, if its argument isn't iterable or a sequence, even if implements __reversed__. The reason why the typetests did not complain here, is because it simply wasn't tested.

This PR makes it so that (presumably) only mypy will incorrectly reject it, due to a bug. But the new tests that I included only fail because of mypy's bug.

So I don't see how deferring this would be any better than mergin this. I believe it's reasonable to ignore mypy's false negatives, since they're directly caused by a known bug.

srittau commented Mar 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

The mypy primer output above shows the real-world impact this change has on a selected number of packages. Each green line is a new mypy warning. As you can see, this change unfortunately leads to an excessive amount of new errors. As far as I understand, many reversed(...) calls now return ...[Never], which leads to all the errors above.

Copy link
Copy Markdown
Contributor Author

@srittau ah ok I see. I blindly assumed that "green is good" for some reason 🤷🏻

Copy link
Copy Markdown
Contributor Author

According to the updated typing spec, it is indeed allowed for __new__ to return some non Self type:
https://typing.readthedocs.io/en/latest/spec/constructors.html#new-method

This comment has been minimized.

Copy link
Copy Markdown

Note that the same was attempted in #13269 so the discussion there may apply here as well.

srittau commented Feb 28, 2025

Copy link
Copy Markdown
Collaborator

At the moment, this doesn't look promising. Unless someone has another good idea, I suggest we close this PR – at least for now.

Copy link
Copy Markdown
Contributor Author

At the moment, this doesn't look promising. Unless someone has another good idea, I suggest we close this PR – at least for now.

Yea, not much movement on the mypy front unfortunately...

Maybe it could help to avoid duplicate PR's if we keep this open? It's also fine to close it if you want, though; whatever floats for boat 🤷🏻.

srittau marked this pull request as draft February 28, 2025 11:16

srittau commented Feb 28, 2025

Copy link
Copy Markdown
Collaborator

I see that it's already marked as deferred. I've also marked it as draft, just so it doesn't stand out as much in the open PRs list.

Copy link
Copy Markdown
Contributor Author

I also rebased onto main, pro forma I guess.

This comment has been minimized.

tungol commented May 27, 2026

Copy link
Copy Markdown
Contributor

Mypy python/mypy#21441 appear to fix the blocker. Once that makes it into a release, this can be revisited.

Copy link
Copy Markdown
Contributor Author

Mypy python/mypy#21441 appear to fix the blocker. Once that makes it into a release, this can be revisited.

python/mypy#21441 was fixed in mypy 2.2.0 :)

Copy link
Copy Markdown
Contributor Author

those blank lines were inserted by pre-commit for some reason 🤷

Copy link
Copy Markdown
Contributor

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

comtypes (https://github.com/enthought/comtypes)
- comtypes/server/register.py:226: error: Incompatible types in assignment (expression has type "Iterator[tuple[int, str]]", variable has type "list[tuple[int, str]]")  [assignment]
+ comtypes/server/register.py:226: error: Incompatible types in assignment (expression has type "reversed[tuple[int, str]]", variable has type "list[tuple[int, str]]")  [assignment]

discord.py (https://github.com/Rapptz/discord.py)
- discord/abc.py:1815: error: Incompatible types in assignment (expression has type "Iterator[MessagePin]", variable has type "list[MessagePin]")  [assignment]
+ discord/abc.py:1815: error: Incompatible types in assignment (expression has type "reversed[MessagePin]", variable has type "list[MessagePin]")  [assignment]

pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/monkeypatch.py:417: error: Unused "type: ignore" comment  [unused-ignore]
+ src/_pytest/monkeypatch.py:422: error: Unused "type: ignore" comment  [unused-ignore]

jinja (https://github.com/pallets/jinja)
+ src/jinja2/utils.py:570: error: Returning Any from function declared to return "Iterator[Any]"  [no-any-return]

jorenham marked this pull request as ready for review July 22, 2026 23:45
srittau removed the status: deferred Issue or PR deferred until some precondition is fixed label Jul 23, 2026

srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks!

srittau merged commit 7f640ab into python:main Jul 23, 2026
73 checks passed
jorenham deleted the re-versed branch July 23, 2026 09:23
cdce8p mentioned this pull request Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

incorrect annotations for builtins.reversed

4 participants


Back | FazBrowse Home | New Git URL