[ty] Filter overloads based on Any / Unknown
#18607
Merged
Conversation
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
Contributor
|
dhruvmanila
force-pushed
the
dhruv/overload-step-5
branch
from
e2b0c19 to
7693df9
Compare
dhruvmanila
force-pushed
the
dhruv/overload-step-5
branch
from
7693df9 to
e77e099
Compare
dhruvmanila
added a commit
that referenced
this pull request
Jun 12, 2025
## Summary This is to support #18607. This PR adds support for generating the top materialization (or upper bound materialization) and the bottom materialization (or lower bound materialization) of a type. This is the most general and the most specific form of the type which is fully static, respectively. More concretely, `T'`, the top materialization of `T`, is the type `T` with all occurrences of dynamic type (`Any`, `Unknown`, `@Todo`) replaced as follows: - In covariant position, it's replaced with `object` - In contravariant position, it's replaced with `Never` - In invariant position, it's replaced with an unresolved type variable (For an invariant position, it should actually be replaced with an existential type, but this is not currently representable in our type system, so we use an unresolved type variable for now instead.) The bottom materialization is implemented in the same way, except we start out in "contravariant" position. ## Test Plan Add test cases for various types.
dhruvmanila
force-pushed
the
dhruv/overload-step-5
branch
2 times, most recently
from
ba64697 to
c5e01dc
Compare
dhruvmanila
force-pushed
the
dhruv/overload-step-5
branch
3 times, most recently
from
47bd939 to
689ca40
Compare
dhruvmanila
changed the title
Any / UnknownAny / Unknown
Member
Author
|
Fixed this. |
dhruvmanila
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
dhruvmanila
force-pushed
the
dhruv/overload-step-5
branch
from
689ca40 to
06b2e88
Compare
carljm
approved these changes
Jun 17, 2025
|
|
||
| # `Any` matches both overloads, but the return types are not equivalent. | ||
| # Pyright and mypy both reveals `str` here. | ||
| reveal_type(f(any)) # revealed: Any |
Contributor
There was a problem hiding this comment.
To be honest, I think we should reveal str here also, even though that's not what the spec says. Perhaps the spec should be changed. It feels like str is just a strictly superior result here.
If the return types don't have a subtype relation, then unioning will create lots of false positives (and break the gradual guarantee), since callers would be forced to handle both possibilities (a OneOf connective would be ideal here.) But when the possible return types do have a subtype relation, there are no false positives created by just returning the wider type.
Member
Author
There was a problem hiding this comment.
I'll create a new tracking issue for this, I think it is a reasonable change but would be also useful to get it into the spec as well.
| from overloaded import f | ||
|
|
||
| def _(list_int: list[int], list_any: list[Any], any: Any): | ||
| reveal_type(f(list_int)) # revealed: A |
Contributor
There was a problem hiding this comment.
Can we also show that passing e.g. list[str] returns str, just to clarify the handling of the generic second overload?
Member
Author
There was a problem hiding this comment.
Huh, it's actually giving me Any instead, let me look at it.
Member
Author
There was a problem hiding this comment.
This is giving me false:
from ty_extensions import static_assert, is_assignable_to
from typing import TypeVar
T = TypeVar("T")
# False
static_assert(is_assignable_to(list[str], list[T]))Shouldn't this be assignable?
Member
Author
|
Ecosystem analysis: Accessing a member of the dictionary when the default is def _(d: dict[str, str], default):
# revealed: Unknown (should be str | None)
reveal_type(d.get("hello", default))I think most of the overloaded function calls are now returning There are multiple instances where it's due to missing features so the argument type would be |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Closes: astral-sh/ty#552
This PR adds support for step 5 of the overload call evaluation algorithm which specifies:
The algorithm works in two parts:
For step 1, the participating parameter indexes are computed by just comparing whether all the parameter types at the corresponding index for all the overloads are gradual equivalent.
The step 2 of the algorithm used is described in this comment.
Test Plan
Update the overload call tests.