-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
PEP 702 (@deprecated): "normal" overloaded methods #18477
base: master
Are you sure you want to change the base?
PEP 702 (@deprecated): "normal" overloaded methods #18477
Conversation
Fixes python#18474 It seems I covered overloaded functions, descriptors, and special methods so far but completely forgot about "normal" methods (thanks to @sobolevn for pointing this out). This addition should do the trick.
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.
Thanks! What about @staticmethod
and @classmethod
?
mypy/checkexpr.py
Outdated
(node is None) | ||
and (member is not None) | ||
and isinstance(object_type, Instance) | ||
and ((symbol := object_type.type.names.get(member)) is not None) |
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.
(node is None) | |
and (member is not None) | |
and isinstance(object_type, Instance) | |
and ((symbol := object_type.type.names.get(member)) is not None) | |
node is None | |
and member is not None | |
and isinstance(object_type, Instance) | |
and (symbol := object_type.type.names.get(member)) is not None |
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.
I will have a look at it later.
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.
It became a one-liner followed by a loop.
Inheritance is also an issue: If the (overloaded) function is defined by a base class of The base class is not directly available. Going through the MRO might be necessary (at least for static methods). |
…ed_overload_item` for static methods
…ods' into narrowing/overloaded_normal_methods # Conflicts: # mypy/checkexpr.py
@sobolevn: |
This comment has been minimized.
This comment has been minimized.
@classmethod | ||
def h(self, v: Union[int, str]) -> A: ... | ||
|
||
class B(A): ... |
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.
Should we also test that A
can have regular method overriden with B
with deprecated case?
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.
I think this is not really necessary. Unless you suggest not reporting a deprecation warning if a base class offers a corresponding not deprecated method. This seems somehow related to #18085, which has not received a response so far. Maybe, because it is a little confusing. Or because the topic confused me...
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Fixes #18474
It seems I covered overloaded functions, descriptors, and special methods so far but completely forgot about "normal" methods (thanks to @sobolevn for pointing this out). This addition should do the trick.