[ty] Narrow mapping patterns in match like isinstance(Mapping)#23462
Merged
sharkdp merged 3 commits intoastral-sh:mainfrom Feb 24, 2026
Merged
[ty] Narrow mapping patterns in match like isinstance(Mapping)#23462sharkdp merged 3 commits intoastral-sh:mainfrom
match like isinstance(Mapping)#23462sharkdp merged 3 commits intoastral-sh:mainfrom
Conversation
Typing conformance resultsNo changes detected ✅ |
|
Memory usage reportMemory usage unchanged ✅ |
Hugo-Polloli
requested review from
AlexWaygood,
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
0 | 1 | 0 |
| Total | 0 | 1 | 0 |
Hugo-Polloli
commented
Feb 21, 2026
| case {}: | ||
| reveal_type(x) # revealed: dict[Unknown, Unknown] | (int & Top[Mapping[Unknown, object]]) | ||
| case _: | ||
| reveal_type(x) # revealed: int & ~Top[Mapping[Unknown, object]] |
Contributor
Author
There was a problem hiding this comment.
The revealed type is semantically correct, but I wonder how useful it is to end-users. It's important to preserve this in our internal representation, but surfacing it directly may be unnecessarily complex and confusing?
Though it would be a separate issue, loosely related to astral-sh/ty#2233 ?
Contributor
There was a problem hiding this comment.
We will ignore negative elements in intersections for a lot of operations on types, so maybe this is fine? I certainly agree that it's not going to be very useful though. I would also agree that we can consider this separately.
sharkdp
approved these changes
Feb 24, 2026
Contributor
sharkdp
left a comment
sharkdp
left a comment
There was a problem hiding this comment.
Thank you very much! Very clear implementation.
| case {}: | ||
| reveal_type(x) # revealed: dict[Unknown, Unknown] | (int & Top[Mapping[Unknown, object]]) | ||
| case _: | ||
| reveal_type(x) # revealed: int & ~Top[Mapping[Unknown, object]] |
Contributor
There was a problem hiding this comment.
We will ignore negative elements in intersections for a lot of operations on types, so maybe this is fine? I certainly agree that it's not going to be very useful though. I would also agree that we can consider this separately.
| def test_match_refutable(x: dict | int) -> None: | ||
| match x: | ||
| case {"k": _}: | ||
| reveal_type(x) # revealed: dict[Unknown, Unknown] | (int & Top[Mapping[Unknown, object]]) |
Contributor
There was a problem hiding this comment.
I wanted to propose that we add
Suggested change
| reveal_type(x) # revealed: dict[Unknown, Unknown] | (int & Top[Mapping[Unknown, object]]) | |
| reveal_type(x) # revealed: dict[Unknown, Unknown] | (int & Top[Mapping[Unknown, object]]) | |
| reveal_type(x["k"]) # revealed: object |
but I noticed that this emits an error at the moment. I think that is a bug separate from this PR though. I'll open an issue.
I initially thought we might gain anything from (additionally) intersecting with a synthetic TypedDict type that has a "k" key, but I guess that wouldn't be correct in general(?), and the permissive dict and Mapping specializations should be enough to support this already.
Contributor
There was a problem hiding this comment.
Thinking a bit more about this, it seems correct that x["k"] emits an error.
from typing import Mapping, reveal_type
def _(obj: object):
if isinstance(obj, Mapping):
reveal_type(obj) # Top[Mapping[Unknown, object]]
obj["some_key"] # ty: errorThe full error message is:
error[invalid-argument-type]: Method
__getitem__of typebound method Top[Mapping[Unknown, object]].__getitem__(key: Never, /) -> objectcannot be called with key of typeLiteral["some_key"]on object of typeTop[Mapping[Unknown, object]]
As we can see, the __getitem__ signature turns into Never -> object when accessed on obj, because key: _KT is in contravariant position, so the top-materialization turns into the bottom-materialization, and Bottom[Unknown] = Never.
def __getitem__(self, key: _KT, /) -> _VT_co: ...In a sense, that is consistent. Top[Mapping[Unknown, object]] represents the infinite union of fully-static Mapping specializations Mapping[str, object] | Mapping[int, object] | …, and since Mapping is invariant in _KT, that union doesn't simplify. Whatever key type we choose to index with, there will always be members in this union that we're not allowed to index into.
So maybe the idea with the synthetic TypedDict is valuable after all?
Member
There was a problem hiding this comment.
So maybe the idea with the synthetic TypedDict is valuable after all?
You might run into issues there, because of the fact that TypedDict types are not subtypes of Top[dict[str, Any]]. In fact, TypedDict types are disjoint from all dict types! (Though we don't implement the disjointness yet.)
@oconnor663 came up with some ingenious solutions to this problem when he implemented TypedDict tagged-union narrowing: we actually intersect with the negation of a synthesized TypedDict that itself has item(s) with negated types in it. But I'm not totally sure whether those same tricks would work here or not
Contributor
There was a problem hiding this comment.
I could be wrong (haven't thought about it carefully in a while) but I'm not sure that TypedDict types are disjoint from dict types? They are generally not assignable in either direction, but that's not the same as being disjoint.
Member
There was a problem hiding this comment.
If a type X is not disjoint from a type Y, that means that it is possible to conceive of a type that would be a subtype of both X and Y. But there can never be any type that is both a subtype of a TypedDict type and a subtype of a dict type, I don't think.
It wouldn't be sound for a type checker to ever consider any given object to simultaneously inhabit a dict type and a TypedDict type. How do you resolve the fact that there are certain method calls that are explicitly allowed by typeshed's dict annotations but explicitly disallowed by the TypedDict spec?
The definition of TypedDictness involves a partial negation of what it means for an object to be a dict.
Contributor
There was a problem hiding this comment.
Ok yeah, you're right. For them to not be disjoint would require allowing a subtype of a TypedDict to gain methods like .clear() -- but that can't be soundly allowed.
Contributor
There was a problem hiding this comment.
I think the other way to handle a pattern like {"k": _} to make a subsequent x["k"] not error would be to use place narrowing on x["k"] rather than narrowing on the type of x.
Contributor
There was a problem hiding this comment.
I didn't invest further in this area so far, because it seems more likely that users would bind the value to a name immediately (case {"k": value}:), instead of relying on x["k"] to be present.
| if kind.is_irrefutable() { | ||
| Truthiness::AlwaysTrue | ||
| } else { | ||
| Truthiness::Ambiguous |
Contributor
There was a problem hiding this comment.
I guess we could be more ambitious here. For example, if the match subject has a TypedDict type where all matched-on keys are marked Required, we could probably return AlwaysTrue. Not sure if it's worth doing that, though.
Member
There was a problem hiding this comment.
I think analysis like that is probably worth doing (see python/typing#2185 for recent discussion), but I would suggest it should be a separate PR to this one
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes astral-sh/ty#2865
Make mapping patterns in
matchnarrow types consistently withisinstance(x, Mapping), including reachability behavior for match arms.Test Plan
Validated in mdtests:
case {}narrowsdict | intthe same way asisinstance(x, Mapping)case {}excludes mapping-like valuescase {"k": _}does not over-narrow away mappings