Skip to content

Modified __add__ method in tuple class #3252

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 30, 2019
Merged

Modified __add__ method in tuple class #3252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22cb7d1
Modified __add__ method in tuple class to allow it to accept tuples w…
msfterictraut Sep 23, 2019
3946c18
Based on PR discussion, added overload for tuple `__add__` method.
msfterictraut Sep 24, 2019
c14076e
Sync'ed changes from builtins.pyi into __builtin__.pyi.
msfterictraut Sep 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions stdlib/2/__builtin__.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,10 @@ 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: ...
@overload
def __add__(self, x: Tuple[_T_co, ...]) -> Tuple[_T_co, ...]: ...
@overload
def __add__(self, x: tuple) -> tuple: ...
def __mul__(self, n: int) -> Tuple[_T_co, ...]: ...
def __rmul__(self, n: int) -> Tuple[_T_co, ...]: ...
def count(self, x: Any) -> int: ...
Expand Down
3 changes: 3 additions & 0 deletions stdlib/2and3/builtins.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,10 @@ 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: ...
@overload
def __add__(self, x: Tuple[_T_co, ...]) -> Tuple[_T_co, ...]: ...
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@overload
def __add__(self, x: tuple) -> tuple: ...
def __mul__(self, n: int) -> Tuple[_T_co, ...]: ...
def __rmul__(self, n: int) -> Tuple[_T_co, ...]: ...
def count(self, x: Any) -> int: ...
Expand Down