Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.9k
Use normalized tuples for fallback calculation#19111
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Use normalized tuples for fallback calculation#19111
Uh oh!
There was an error while loading.Please reload this page.
Conversation
According tomypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
@ilevkivskyi sorry to bother you again, I'd appreciate a review - this PR fixes a crash and you should be well familiar with the affected part. Thanks! |
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, unfortunately this whole normalization story is a bit tricky (partially because of design limitations ofsemanal_typeags.py). FWIW I think for now some "reactive" bug fixing is OK, but at some point we may need to think about tightening this a bit more.
| # https://github.com/python/mypy/issues/19105 | ||
| from typing import Unpack | ||
| class A(tuple[Unpack[tuple[int]]]): ... |
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.
Could you please also add a test for
classC(tuple[Unpack[tuple[int, ...]]]): ...
(unless there is already such test case)? I am not sure we properly handle such types. Also maybe test not just class creation, but some tuple ops as well, such as e.g. unpacking etc.
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.
Surprisingly we don't have such a test too, but it was already working before my patch -*tuple[int, ...] is not a trivial unpack. But your feeling is overall correct - I'm able to make it crash with
a:Atuple(a)
and also get a terrible diagnostic on
b:B_,=b# E: Variadic tuple unpacking requires a star target
despite B clearly not being variadic. I'll move this to draft for now, there's little benefit from merging a crash fix that causes a next one immediately...
Fixes#19105. I haven't checked this with namedtuples magic, nor am I certain that this is the best way.