Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork3.1k
[PEP 695] Allow Self return types with contravariance#17786
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Fix variance inference in this fragment from a typing conformance test:```class ClassA[T1, T2, T3](list[T1]): def method1(self, a: T2) -> None: ... def method2(self) -> T3: ...```Previously T2 was incorrectly inferred as invariant due to `list` havingmethods that return `Self`. Be more flexible with return types to allowinferring contravariance for type variables even if there are `Self` returntypes, in particular.We could probably make this even more lenient, but after thinking aboutthis for a while, I wasn't sure what the most general rule would be, soI decided to just make a tweak to support the likely most common use case(which is probably actually not that common either).Link to conformance test:https://github.com/python/typing/blob/main/conformance/tests/generics_variance_inference.py#L15C1-L20C12
This comment has been minimized.
This comment has been minimized.
JukkaL commentedSep 19, 2024
The build failure looks like a flake. |
According tomypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
| a = b # E: Incompatible types in assignment (expression has type "Invariant[int]", variable has type "Invariant[object]") | ||
| if int(): | ||
| b = a # E: Incompatible types in assignment (expression has type "Invariant[object]", variable has type "Invariant[int]") | ||
| b = a |
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 think this is not right. It happens because with current implementation we check return type afterbind_self(), so we can't distinguish situation with an explicitSelf from situation where return type just happens to (accidentally) match current class. I understand thatSelf-types are inherently (slightly) unsafe, but I don't want this unsafety to "leak" to code that doesn't useSelf.
I think it makes sense to limit the return type erasure only to situations where the original callable type contained an explicitSelf type (or the "old style" self type).
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.
Btw when looking at this I found a bug in variance inference that affects protocols as well. In particular, all three examples above should be inferred as covariant (or even bivariant, but we don't have that).
Also to clarify, the unsafety I meant above relates to examples like
classC[T]:defmeth(self,x:T,y:C[T])->C[T]: ...
that shouldnot be contravariant, while it seems to me with the proposed PR it will be. But after playing with this more, inference doesn't behave as it should on master either. This needs some cleanup.@JukkaL we can discuss this in our meeting on Tuesday.
ilevkivskyi left a comment
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.
As per our discussion, you can merge this now (to unblock making PEP 695 official), and I will do some variance inference overhaul later.
Fix variance inference in this fragment from a typing conformance test:
Previously T2 was incorrectly inferred as invariant due to
listhaving methods that returnSelf. Be more flexible with return types to allow inferring contravariance for type variables even if there areSelfreturn types, in particular.We could probably make this even more lenient, but after thinking about this for a while, I wasn't sure what the most general rule would be, so I decided to just make a tweak to support the likely most common use case (which is probably actually not that common either).
Link to conformance test:
https://github.com/python/typing/blob/main/conformance/tests/generics_variance_inference.py#L15C1-L20C12