[ty] Use specialized parameter type for overload filter #19964
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
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
Contributor
|
dhruvmanila
force-pushed
the
dhruv/use-specialized-parameter-type
branch
from
628363b to
5ebe3e6
Compare
This reverts commit 5ebe3e6.
Contributor
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
14 | 0 | 0 |
invalid-assignment |
9 | 0 | 0 |
no-matching-overload |
7 | 2 | 0 |
possibly-unbound-attribute |
0 | 0 | 4 |
call-non-callable |
3 | 0 | 0 |
unresolved-attribute |
2 | 0 | 0 |
invalid-return-type |
1 | 0 | 0 |
missing-argument |
1 | 0 | 0 |
possibly-unbound-implicit-call |
0 | 0 | 1 |
redundant-cast |
1 | 0 | 0 |
| Total | 38 | 2 | 5 |
Member
Author
This is interesting, it seems that a decorator that returns a callable on a method is removing the fact that the original function is an instance method: def __rmatmul__(self, other: MatrixBase) -> MatrixBase:
self, other, T = _unify_with_other(self, other)
if T != "is_matrix":
return NotImplemented
# This is the revealed type with the `call_highest_priority` decorator
# ty: Revealed type: `Unknown | ((Unknown, Unknown, /) -> Unknown)` [revealed-type]
# Pyright: Type of "self.__rmul__" is "(MatrixBase | Expr | complex) -> MatrixBase"
return reveal_type(self.__rmul__)(other)
# And, this is without the decorator
# ty: Revealed type: `Unknown | (bound method MatrixBase.__rmul__(other: MatrixBase | Unknown) -> MatrixBase)` [revealed-type]
@call_highest_priority('__mul__')
def __rmul__(self, other: MatrixBase | SExpr) -> MatrixBase:
return self.rmultiply(other)This wasn't an easy before this PR because the call to an overloaded function |
dhruvmanila
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
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#669
(This turned out to be simpler that I thought :))
Test Plan
Update existing test cases.
Ecosystem report
Most of them are basically because ty has now started inferring more precise types for the return type to an overloaded call and a lot of the types are defined using type aliases, here's some examples:
Details
This is accurate now that we infer the type as
Literal[42]instead ofUnknown(Pyright infers it asint)Same as above where ty is now inferring a more precise type like
Unknown | ndarray[tuple[int, int], <class 'float'>]instead of justUnknownas beforeThis requires support for type aliases to match the correct overload.
Same as above, this requires support for type aliases to match the correct overload.
This is correct.
Most of them are correct except for the last two diagnostics which I'm not sure
what's happening, it's trying to index into an
np.ndarraytype (which isinferred correctly) but I think it might be picking up an incorrect overload
for the
__getitem__method.Scipy's diagnostics also requires support for type alises to pick the correct overload.