-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Fix handling of named tuples in class match pattern #18663
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
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.
Nice to see a fix with negative lines of (non-test) code! Left an idea about an additional test case (assuming one doesn't already exist), looks good otherwise.
def f(t: T) -> None: | ||
match t: | ||
case T([K() as k]): | ||
reveal_type(k) # N: Revealed type is "Tuple[builtins.int, fallback=__main__.K]" |
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 wonder if there is a test case like this, which would cover structural matching of named tuples:
from typing import NamedTuple
class N(NamedTuple):
x: int
y: str
a = N(1, "a")
match a:
case [x, y]:
reveal_type(x)
reveal_type(y)
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.
No, I don't see such test, will add one.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Previously a code path was introduced that made fallback a subtype of its tuple type for non-generic tuples, while the intention was to cover `tuple[Any, ...]` and similar. I add a unit test + some refactoring to make this mistake much harder in future. This may need to wait for #18663 to avoid "regressions" (the other fix needed to avoid "regressions" is already merged).
Fixes python#15299 The fix is straightforward, named tuples should be properly represented as tuples with fallback, not as instances.
Fixes python#15299 The fix is straightforward, named tuples should be properly represented as tuples with fallback, not as instances.
Fixes python#15299 The fix is straightforward, named tuples should be properly represented as tuples with fallback, not as instances.
Previously a code path was introduced that made fallback a subtype of its tuple type for non-generic tuples, while the intention was to cover `tuple[Any, ...]` and similar. I add a unit test + some refactoring to make this mistake much harder in future. This may need to wait for python#18663 to avoid "regressions" (the other fix needed to avoid "regressions" is already merged).
Fixes #15299
The fix is straightforward, named tuples should be properly represented as tuples with fallback, not as instances.