parameter specification/type variable tuple variance#2215
Conversation
5f8e0cc to
d9d82c9
Compare
81ba400 to
54ad5e1
Compare
JelleZijlstra
left a comment
There was a problem hiding this comment.
I like adding this if we can get it specified nicely, but this PR is not ready; the proposed test is incorrect.
Also, https://typing.python.org/en/latest/spec/generics.html#paramspec-variables still says variance on ParamSpec is unsupported; this should be updated.
I'd also like to see an implementation in at least one type checker, even if only as a draft PR, so we can be confident this is something that can be feasibly implemented.
|
I personally would also like to see tests added for param spec variance inference, since that would probably need to be supported as well. |
|
The test cases do have variance inference, though as I noted some of the cases are wrong. But it would probably be useful to have a few more cases, and I'd recommend putting the tests for paramspec variance in their own file so we can track type checker support more precisely. |
I have wip support in PyCharm, but I could also add it to basedpyright it was very straightforward to implement |
Oh right, sorry I didn't see them, because I thought they would be in a different file. I'm also very much +1 on putting those tests into a different file (for example |
8f30004 to
117964e
Compare
7a21592 to
5788125
Compare
|
support has landed in pycharm and cpython |
|
https://typing.python.org/en/latest/spec/generics.html#paramspec-variables stills says that we don't support variance in ParamSpec, that should be fixed in this PR. |
|
Also can you open an issue on python/typing-council asking for a formal pronouncement? |
5788125 to
f23d8ae
Compare
|
Noticed some more issues:
|
not |
414c697 to
02c4628
Compare
|
I don't think that's right. The form we want here should be a supertype of every other possible value, and something like a one-parameter callable is not a subtype of |
|
In ty we have a type that we spell as |
|
We only need assignability not subtyping here, so I think |
carljm
left a comment
There was a problem hiding this comment.
Thanks! Definitely support this change.
|
|
||
| an ``object`` instance for a type variable. | ||
| a ``*tuple[object, ...]`` value for a type variable tuple. | ||
| a ``...`` value for a parameter specification. |
There was a problem hiding this comment.
I don't think using ... here is sound. Because it is both assignable-to and assignable-from any paramspec, it means that a case like the ContravariantParamSpec test will wrongly infer the paramspec as covariant (because lower is assignable to upper) before it even checks the contravariant direction.
I don't think there is any way to make the variance-inference algorithm described here correct and support inference of ParamSpec variance, without introducing the new concept of a "top signature".
(I'm also not sure how much value there is in including this particular variance-inference algorithm in the spec. I think it describes pyright's algorithm. I know that ty and pyrefly do not use this algorithm. I don't know about mypy or zuban.)
There was a problem hiding this comment.
(I'm also not sure how much value there is in including this particular variance-inference algorithm in the spec. I think it describes pyright's algorithm. I know that ty and pyrefly do not use this algorithm. I don't know about mypy or zuban.)
i agree, but i think this point is out of the scope of this change
There was a problem hiding this comment.
We need to describe the algorithm somehow, don't we? Other type checkers use a different algorithm in practice, but to be conformant they need to follow an algorithm that has the same results as that described in the spec.
There was a problem hiding this comment.
Depending on the situation I think it can be fine (or preferable) for the spec to describe the required results without specifying a particular algorithm to reach them. In this case I'm not sure that would be any simpler (and in any case I don't think we should remove this algorithm in this PR.)
I think for this PR we can fairly simply describe the nature of the top signature (the union of all possible callable signatures) without having to go too deep into it.
There was a problem hiding this comment.
i removed the ...
| @@ -0,0 +1,53 @@ | |||
| """ | |||
| Tests variance of TypeVarTuple. | |||
There was a problem hiding this comment.
Here too, I think we should have tests for invariant TypeVarTuple.
There was a problem hiding this comment.
i added tests for invariant tvt
But we also need assignability to fail in the "wrong" direction, and this workaround fails that requirement. |
ec71bfe to
7cfeee0
Compare
7cfeee0 to
1d3e04d
Compare
## Summary This PR adds `covariant`, `contravariant`, and `infer_variance` support to `ParamSpec`. See: python/typing#2215. See: #24479 (review).
|
i've pushed changes, it's good to review again |
JelleZijlstra
left a comment
There was a problem hiding this comment.
Agree with this idea, we might have to tweak the exact PR a bit more.
## Summary This PR adds `covariant`, `contravariant`, and `infer_variance` support to `ParamSpec`. See: python/typing#2215. See: astral-sh#24479 (review).
davidhalter
left a comment
There was a problem hiding this comment.
I like the general approach. I think it would also be useful to have at least one type checker with a proper implementation of this new feature.
|
|
||
| an ``object`` instance for a type variable. | ||
| a ``*tuple[object, ...]`` value for a type variable tuple. | ||
| a for a parameter specification, a 'top signature' value, i.e. a type |
There was a problem hiding this comment.
I assume a for a is not wanted? The whole paragraph feels a bit weird and the formatting might be a bit off.
There was a problem hiding this comment.
addressed points 1 and 3. do you have a suggestion on how to improve the wording?
|
|
||
|
|
||
| class Array3(Generic[*Ts1, *Ts2]): # E | ||
| class Array3[*Ts1, *Ts2]: # E |
There was a problem hiding this comment.
Personally I would prefer to the both the new and the old syntax.
see the mr description, it's implemented in pycharm and basedpyright |
fdb31bb to
94b1653
Compare
fixup! fixup! specify that parameter specification should have variance
94b1653 to
7a4c793
Compare
discussion: https://discuss.python.org/t/parameter-specifications-should-have-variance/106452
typing.TypeVarTuplecpython#148212typing.TypeVarTuple: add bound/variance properties from 3.15 typeshed#15670 / Add Python 3.15 typing updates typeshed#15725