Uh oh!
There was an error while loading. Please reload this page.
Improve match statement union narrowing/inference - #17600
Conversation
This comment has been minimized.
This comment has been minimized.
Hnasar
left a comment
There was a problem hiding this comment.
Nice!
Does your fix address any of the other match statement issues?
https://github.com/python/mypy/issues?q=sort%3Aupdated-desc+is%3Aopen+label%3Atopic-match-statement
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hashem <Hnasar@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
bergkvist
commented
Aug 11, 2024
From reading through the issues, I believe it should at least solve these:
I notice there are a bunch of match exhaustiveness issues with narrowing of the type passed on to the next match branches, which this PR does not fix. |
This comment has been minimized.
This comment has been minimized.
hjwp
commented
Aug 21, 2024
maybe it would address this too? #16835 |
bergkvist
commented
Aug 21, 2024
I don't think so, as I return the "current type" as the "rest type", which means the remainder/rest type (which is what gets passed on to the next match case) doesn't get narrowed properly yet. I have yet to figure out how to do this. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
smurzin
commented
Jan 22, 2025
Hi @bergkvist@Hnasar are there any blockers for landing this PR? |
I don't think this is solving the issue in my use case involving TypedDicts: I have: matchnode:
case {"type": ("assign"|"cond_assign") asop, "key": key, "value": value}:
kbs=key.encode()
vbs=value.encode()
...where I tried removing the disjunction in the |
Hnasar
commented
Apr 9, 2025
@bergkvist Sorry for the late reply, but can you rebase this on the latest mypy, then we can ping one of the real maintainers to get someone with commit privileges to take a look. |
hauntsaninja
left a comment
There was a problem hiding this comment.
Thanks for working on this! Added two comments, would also be great to add test cases that can detect the effect of those two suggestions
| typ, _, capture = self.accept(o, t) | ||
| if not is_uninhabited(get_proper_type(typ)): | ||
| union_items.append(typ) | ||
| union_captures.update(capture) |
There was a problem hiding this comment.
hmm shouldn't the values in union_captures be a union of all the things, instead of potentially clobbering?
| union_items.append(typ) | ||
| union_captures.update(capture) | ||
| rest_items: list[Type] = [] |
There was a problem hiding this comment.
iiuc this should use conditional_types_with_intersection
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
I realised there is a simpler and more complete approach than the one taken in python#19600. This adds the new "Step 2" to the original code. Best way to review the diff is probably to check it out and review it squashed with python#19600 , but really the net logic change is just "Step 2" Fixespython#15190Fixespython#17549Fixespython#17600 Mostly fixespython#18039 Helps with things in python#19081
Improve inference/narrowing support for union types in match statements.
Fixes#17549
Before:
After:
Related: