-
-
Notifications
You must be signed in to change notification settings - Fork 3k
PEP 702 (@deprecated): handle "combined" overloads #19626
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): handle "combined" overloads #19626
Conversation
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.
LG, but I have couple questions/suggestions.
mypy/checkexpr.py
Outdated
| if unioned_return: | ||
| returns, inferred_types = zip(*unioned_return) | ||
| returns = tuple(u[0] for u in unioned_return) | ||
| inferred_types = tuple(u[1] for u in unioned_return) |
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 you should use regular list comprehensions for these, not tuple(...), they are converted to lists few lines below anyway (and then you can remove the list(...) call below).
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.
Seems I wanted to stick to the functionality of zip as close as possible, which is, in fact, not necessary. I changed it.
mypy/checkexpr.py
Outdated
| else: | ||
| inferred_result = None | ||
| if unioned_result is not None: | ||
| for inferred_type in inferred_types: |
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 am surprised that mypy doesn't yell at you that inferred_types may be undefined.
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.
Don't be surprised, we don't have possibly-undefined enabled for selfcheck for some reason. There were ~50 violations last time I tried to enable it.
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 now initialise inferred_types with None and make an is None check here. It looks a little redundant to me, but maybe better than adding the 51st violation.
|
Thanks for the review! |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
This change is taken from #18682. The new code and the tests are unmodified. I only had to remove two now unnecessary calls of
warn_deprecatedwhich were introduced after opening #18682.