Modified __add__ method in tuple class #3252
Conversation
…ith different generic parameter types. This allows, for example: a = (1, ) b = a + (2.4, )
| @@ -867,7 +867,7 @@ class tuple(Sequence[_T_co], Generic[_T_co]): | |||
| def __le__(self, x: Tuple[_T_co, ...]) -> bool: ... | |||
| def __gt__(self, x: Tuple[_T_co, ...]) -> bool: ... | |||
| def __ge__(self, x: Tuple[_T_co, ...]) -> bool: ... | |||
| def __add__(self, x: Tuple[_T_co, ...]) -> Tuple[_T_co, ...]: ... | |||
There was a problem hiding this comment.
I think it would make more sense to use a second type variable here und return a tuple that is an union of both. I have not checked, but the current version should just return a Tuple[int, ...] for your example.
There was a problem hiding this comment.
I'm unclear on what you mean by "union of both". Are you proposing something like this?
def __add__(self, x: Tuple[_T2_co, ...]) -> Tuple[Union[_T_co, _T2_co], ...]: ...
That doesn't look right to me. Maybe I'm misunderstanding.
There was a problem hiding this comment.
That is what I meant, but it might not be the right solution. Let's think this through: typing basically supports two kinds of tuples: "immutable lists" (PEP 484 calls them "arbitrary-length homogeneous tuples", but that's a mouthful), e.g. Tuple[T, ...] and "real tuples", e.g. Tuple[int, str, float]. The current stub only supports the former case, while your example would need the latter.
I don't think we have any way to support the latter case and keep type safety. Tuple[...] is not supported syntax according to PEP 484 and mypy complains about it. It's basically a one-element tuple, but ... is not a valid type.
My suggestion above is not ideal, because it will allow any type at any position and would mean the user needed to add type check when accessing them. E.g. x[0] in x = ("",) + (3,) would be Union[str, int] when it is obviously str.
The best I can think of is to add an overload like def __add__(self, x: tuple) -> tuple: ..., which will not keep any element types, but at least does accept your example. All elements of the resulting tuple would be Any.
There was a problem hiding this comment.
Thanks for the explanation. Yeah, since there's currently no way to represent the concatenation of two homogeneous tuples, the best we can do is to fall back on an untyped tuple. I suppose we could consider ways to augment the standard (e.g. a spread/unpack operator that can be used within type arg lists), but that's a longer discussion.
|
Maybe it would also make sense to just special-case int/float tuples. |
There was a problem hiding this comment.
LGTM, but I'd like to hear the opinion of @JelleZijlstra or someone else from the mypy team.
There was a problem hiding this comment.
LGTM (but you will need to copy the change to __builtins__.pyi). Hopefully @JelleZijlstra will have time to get this through the sync.
|
I'm not sure what you mean by "copy the change to |
|
For various reasons we need the contents of stdlib/2and3/builtins.pyi and
stdlib/2/__builtin__.pyi to be identical. In the past we used a symlink but
that doesn't always work on Windows, so now you must manually update both,
and we have a test that checks if they are identical.
|
|
Got it — thanks for the explanation. I've updated |
|
Looks like the mypy tests need to be tweaked for this change. I'm not sure how you normally coordinate dependent changes between the mypy and typeshed repos. Could someone familiar with the process help resolve? |
|
Yeah, this happens occasionally, and it's a pain. :-) IIRC you have to prepare a PR for mypy that adjusts the tests there, link the two tests to each other via cross-references in GitHub, and then someone with commit privileges in both repos will merge both, ignoring the test failures. After landing both I believe it will also be necessary to do a third PR, a so-called "typeshed sync" where we move the commit hash of the typeshed submodule in the mypy repo. |
|
I can do this tomorrow morning if no one else will do it before. |
|
I can do it shortly (together with the other mypy/typeshed sync I promised to do). |
Great, thank you! 👍 |
Needed for python/typeshed#3252. Tested by pulling in the typeshed branch in the subproject.
|
Just to summarize for myself, I need to merge the following:
A few of these still have CI running, so I'll wait for those to finish in case something unexpected is wrong. Afterwards, I'll merge all of the open PRs, which will break both mypy and typeshed CI, and then do a typeshed sync in mypy to get things back to green. |
Needed for python/typeshed#3252. Tested by pulling in the typeshed branch in the subproject.
Excluding python/typeshed#3252 GitOrigin-RevId: f6d802b1a50d6071f1a1027017750e4dc65ece26
Excluding python/typeshed#3252 GitOrigin-RevId: 705c358d0112ca475be82659745a1190dd5410d3
Excluding python/typeshed#3252 GitOrigin-RevId: bd535fbff565aef2ba42fd99a7d4a5c7f5c770a8
Modified add method in tuple class to allow it to accept tuples with different generic parameter types. This allows, for example:
a = (1, )
b = a + (2.4, )