Skip to content

Conversation

@maffoo
Copy link
Contributor

@maffoo maffoo commented Jun 2, 2021

Issue reference

Fixes #525

Changes

This adds overloaded type definitions for zip_equal and zip_offset, following the standard-library type definitions for zip (2.x, 3.x). This creates special cases for 1-5 iterable arguments, and a fallback for 6 or more arguments or if called with varargs. This gives better type information in common situations when zipping a fixed number of iterables.

This follows the standard-library type definitions[1,2] for `zip` that
have special overloads for cases with 1-5 arguments, and a fallback
overload for 6 or more arguments (which also applies if called with
explicit varargs).

[1] https://github.com/python/typeshed/blob/master/stdlib/builtins.pyi#L1310
[2] https://github.com/python/typeshed/blob/master/stdlib/%40python2/builtins.pyi#L1030
@bbayles
Copy link
Collaborator

bbayles commented Jun 3, 2021

Thanks for the PR.

The argument for following what the standard library does is strong. However, I think I object on aesthetic grounds. Repeating the same bit of line noise six times? This is Python?

I could see the argument for "one, two, many" perhaps - if you want to resubmit that, I'll merge it?

@maffoo
Copy link
Contributor Author

maffoo commented Jun 3, 2021

I agree it's ugly, but it's also "just" type definitions so doesn't pollute the code itself. But I'll defer to you if you'd like to keep just the first two type specializations. The particular case where I ran into this in my code was zipping two iterables, so keeping just the two-arg case would be sufficient for what I need right now. And I suspect zipping two iterables is the most common case so that might cover other people's needs as well.

@maffoo
Copy link
Contributor Author

maffoo commented Jun 3, 2021

Note: mypy added type defs for zip up to 5 args here: python/typeshed#1041. GvR commented there that this could eventually be superseded by variadic generics: python/typing#193.

@bbayles
Copy link
Collaborator

bbayles commented Jun 4, 2021

But I'll defer to you if you'd like to keep just the first two type specializations.

Yes please!

Copy link
Collaborator

@bbayles bbayles left a comment

Choose a reason for hiding this comment

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

Check out the CI failures on the type changes?

@bbayles bbayles merged commit f32cb4e into more-itertools:master Jun 8, 2021
@jjjamie
Copy link

jjjamie commented Jun 24, 2021

FWIW, I'd also prefer to keep the typedefs for >2 iterables. This fixes Pycharm linter errors for zipping two iterables of different types but it'll still complain about 3+.

I would prefer some extra (well motivated, correct ;)) noise in a pyi file to false errors highlighted in yellow every time I read my code. (Variadic generics haven't appeared in the 4 years since the GvR comment on python/typeshed#1041 - not sure it's worth waiting for them!)

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 this pull request may close these issues.

zip_equal and zip_offset lose type information

3 participants