Skip to content

Regression in determining return type of zip() on tuples #4226

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mthuurne opened this issue Jun 11, 2020 · 6 comments · Fixed by #4254
Closed

Regression in determining return type of zip() on tuples #4226

mthuurne opened this issue Jun 11, 2020 · 6 comments · Fixed by #4254

Comments

@mthuurne
Copy link
Contributor

On the following code:

z = zip((1, 2, 3), ('a', 'b', 'c'))
reveal_type(z)

mypy 0.770 will output:

Revealed type is 'typing.Iterator[Tuple[builtins.int*, builtins.str*]]

while mypy 0.780 will output:

Revealed type is 'Tuple[builtins.tuple[builtins.object*], builtins.tuple[builtins.object*], builtins.tuple[builtins.object*]]'

The output from 0.770 is both more correct (since the zip object is not a tuple) and more useful (since the types of the elements are known).

The fix for python/mypy#8454 adds additional overloaded definitions for zip(*...) in typeshed, which in itself is useful. However, for some reason mypy seems to prefer the new 'star' definitions to the older and more correct 'non-star' definitions, which causes this regression.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jun 11, 2020

We should probably move this to typeshed, I think mypy is doing the right thing here. mypy will usually pick the first matching variant and complicating that logic too much will make mypy unpredictable in ways that are bad for users.

We could patch the situation by adding even more overloads (and removing a few of the longer heterogeneous overloads added in #3830). I think the following could actually work well in practice (it should make both the above and the original python/mypy#8454 work):

# Some overloads to better support zipping heterogeneous tuples
@overload
def zip(__tuple1: Tuple[_T1, _T2], __tuple2: Tuple[_T3, _T4]) -> Tuple[Tuple[_T1, _T3], Tuple[_T2, _T4]]: ...  # type: ignore
@overload
def zip(__tuple1: Tuple[_T1, _T2], __tuple2: Tuple[_T3, _T4], __tuple3: Tuple[_T5, _T6]) -> Tuple[Tuple[_T1, _T3, _T5], Tuple[_T2, _T4, _T6]]: ...  # type: ignore
@overload
def zip(__tuple1: Tuple[_T1, _T2, _T3], __tuple2: Tuple[_T4, _T5, _T6]) -> Tuple[Tuple[_T1, _T4], Tuple[_T2, _T5], Tuple[_T3, _T6]]: ...  # type: ignore
@overload
def zip(__tuple1: Tuple[_T1, _T2, _T3], __tuple2: Tuple[_T4, _T5, _T6], __tuple3: Tuple[_T7, _T8, _T9]) -> Tuple[Tuple[_T1, _T4, _T7], Tuple[_T2, _T5, _T8], Tuple[_T3, _T6, _T9]]: ...  # type: ignore
@overload
def zip(*iterables: Tuple[_T1, _T2]) -> Tuple[Tuple[_T1, ...], Tuple[_T2, ...]]: ...  # type: ignore
@overload
def zip(*iterables: Tuple[_T1, _T2, _T3]) -> Tuple[Tuple[_T1, ...], Tuple[_T2, ...], Tuple[_T3, ...]]: ...  # type: ignore
...  # existing iterable-based definitions

But given that the return type is a lie and that the original patch evidently caused issues I didn't foresee, maybe we should just revert #3830

Would appreciate opinions!

@ilevkivskyi
Copy link
Member

We should probably move this to typeshed [...]

OK, I am moving this issue to typeshed now.

@ilevkivskyi ilevkivskyi transferred this issue from python/mypy Jun 12, 2020
@mthuurne
Copy link
Contributor Author

I don't think there is an easy solution.

Reverting would fix my use case, but people using zip() on heterogeneous tuples would then have to add extra annotations or casts to their code.

Tweaking overlapping rules, as @hauntsaninja proposed above, might work for small inputs, but I think any star rule will be matching too much and forcing the types to object. And then there is also the point that those rules are technically lies that might hide bugs, for example the returned zip object does not support len(), while Tuple does.

Having more advanced rules for handling overlapping overload declarations is likely to indeed get unpredictable if it ends up being some kind of scoring system. And I can't think of anything simpler that still solves the issue.

A special case or plugin in mypy specifically for zip would work, but then someone would have to be willing to invest time into that.

@JelleZijlstra
Copy link
Member

The zip change also caused a performance regression in mypy (python/mypy#9016). Maybe reverting is the right call? The old behavior maybe caused fewer issues.

@staticdev
Copy link

I also got a problem with that and opened a question on SO.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jun 26, 2020

staticdev's issue is unrelated, see python/mypy#9048

We reverted #3830 in #4254, so mthuurne's issue should be fixed. If more people report issues like python/mypy#8454, we could reconsider the overloads or maybe preferably write a mypy plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants