Repository navigation
Simplify documentation of itertools.zip_longest #75453
Description
Activity
raphaelm commented
on Aug 24, 2017 raphaelmmannequinMannequinAuthorThe documentation given for itertools.zip_longest contains a "roughly equivalent" pure-python implementation of the function that is intended to help the user understand what zip_longest does on a functional level.
However, the given implementation is very complicated to read for newcomers and experienced Python programmers alike, as it uses a custom-defined exception for control flow handling, a nested function, a condition that always is true if any arguments are passed ("while iterators"), as well as two other non-trivial functions from itertools (chain and repeat).
For future reference, this is the currently given implementation:
def zip_longest(*args, **kwds): # zip_longest('ABCD', 'xy', fillvalue='-') --> Ax By C- D- fillvalue = kwds.get('fillvalue') iterators = [iter(it) for it in args] while True: exhausted = 0 values = [] for it in iterators: try: values.append(next(it)) except StopIteration: values.append(fillvalue) exhausted += 1 if exhausted < len(args): yield tuple(values) else: break
This is way more complex than necessary to teach the concept of zip_longest. With this issue, I will submit a pull request with a new example implementation that seems to be the same level of "roughly equivalent" but is much easier to read, since it only uses two loops and now complicated flow
def zip_longest(*args, **kwds): # zip_longest('ABCD', 'xy', fillvalue='-') --> Ax By C- D- fillvalue = kwds.get('fillvalue') iterators = [iter(it) for it in args] while True: exhausted = 0 values = [] for it in iterators: try: values.append(next(it)) except StopIteration: values.append(fillvalue) exhausted += 1 if exhausted < len(args): yield tuple(values) else: break
Looking at the C code of the actual implementation, I don't see that any one of the two implementations is obviously "more equivalent". I'm unsure about performance -- I haven't tried them on that but I don't think that's the point of this learning implementation.
I ran all tests from Lib/test/test_itertools.py against both the old and the new implementation. The new implementation fails at 3 tests, while the old implementation failed at four. Two of the remaining failures are related to TypeErrors not being thrown on invalid input, one of them is related to pickling the resulting object. I believe all three of them are fine to ignore in this sample, as it is not relevant to the documentation purpose.
Therefore, I believe the documentation should be changed like suggested. I'd be happy for any feedback or further ideas to improve its readability!
raphaelm commented
on Aug 24, 2017 raphaelmmannequinMannequinAuthorI just noticed that in my post I accidentally pasted MY implementation twice instead of the old one, sorry for that. Here's the old one for quick comparison:
class ZipExhausted(Exception): pass def zip_longest(*args, **kwds): # zip_longest('ABCD', 'xy', fillvalue='-') --> Ax By C- D- fillvalue = kwds.get('fillvalue') counter = len(args) - 1 def sentinel(): nonlocal counter if not counter: raise ZipExhausted counter -= 1 yield fillvalue fillers = repeat(fillvalue) iterators = [chain(it, sentinel(), fillers) for it in args] try: while iterators: yield tuple(map(next, iterators)) except ZipExhausted: pass
Thanks for wanting to improve the documentation.
Raymond will address this definitively, but unless I'm mistaken part of the purpose of the examples is to show how the various itertools can be used. If that is true, then in the context of the overall itertools documentation I think the current example has more teaching value than your suggested revision.
raphaelm commented
on Aug 25, 2017 raphaelmmannequinMannequinAuthorWell, I could think of a way to still use repeat() here that also is pretty clean except for the fact that it fails if all inputs to zip_longest are repeat() iterators themselves (which would here lead to an empty iterator while it would otherwise lead to an infinite one):
def zip_longest(*args, **kwds): # zip_longest('ABCD', 'xy', fillvalue='-') --> Ax By C- D- fillvalue = kwds.get('fillvalue') iterators = [iter(it) for it in args] while True: values = [] for i, it in enumerate(iterators): try: values.append(next(it)) except StopIteration: values.append(fillvalue) iterators[i] = repeat(fillvalue) if all(isinstance(it, repeat) for it in iterators): break else: yield tuple(values)
Keeping chain() in use here just for the sake of using it is not worth it, I believe.
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: