-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Allow union-with-callable attributes to be overridden by methods #18018
Allow union-with-callable attributes to be overridden by methods #18018
Conversation
This comment has been minimized.
This comment has been minimized.
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Could you add a test case with this error code enabled: https://github.com/python/mypy/pull/16399/files
93f6bf7
to
a9c5dac
Compare
Diff from mypy_primer, showing the effect of this PR on open source code: packaging (https://github.com/pypa/packaging)
+ src/packaging/specifiers.py:248: error: Unused "type: ignore" comment [unused-ignore]
steam.py (https://github.com/Gobot1234/steam.py)
- steam/models.py:239: error: Signature of "url" incompatible with supertype "_IOMixin" [override]
- steam/models.py:239: note: Superclass:
- steam/models.py:239: note: str | _ReadOnlyProto[str]
- steam/models.py:239: note: Subclass:
- steam/models.py:239: note: str
- steam/reaction.py:236: error: Signature of "url" incompatible with supertype "_IOMixin" [override]
- steam/reaction.py:236: note: Superclass:
- steam/reaction.py:236: note: str | _ReadOnlyProto[str]
- steam/reaction.py:236: note: Subclass:
- steam/reaction.py:236: note: str
- steam/reaction.py:273: error: Signature of "url" incompatible with supertype "_IOMixin" [override]
- steam/reaction.py:273: note: Superclass:
- steam/reaction.py:273: note: str | _ReadOnlyProto[str]
- steam/reaction.py:273: note: Subclass:
- steam/reaction.py:273: note: str
- steam/reaction.py:312: error: Signature of "url" incompatible with supertype "_IOMixin" [override]
- steam/reaction.py:312: note: Superclass:
- steam/reaction.py:312: note: str | _ReadOnlyProto[str]
- steam/reaction.py:312: note: Subclass:
- steam/reaction.py:312: note: str
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I kind of want to try ignore_pos_arg_names=False
. This is something we probably want to eventually do anyway and should be fairly harmless in this case. But it's likely wiser to be consistent.
I was thinking about this PR some more and I think it introduces some unsoundness. We should have a Liskov error in the following:
Input to setter should be wider, output of getter should be narrower. |
Hmm, I think that setup is currently caught by mutable-override, is it not? Is the behavior after this PR different from what mypy does for this setup? class X:
x: bool | None
class Y(X):
x: bool |
Oh looks like I must have messed up my test. It does error with mutable-override, sorry for the noise! |
I think what happened is I messed up the branches I tested. I think I probably did this, which mypy doesn't error on:
This would be unsound, except that mypy doesn't support this at all and will error if you attempt to set |
Fixes #12569
Before
After