python / cpython

The Python programming language
https://www.python.org
Other
63.15k stars 30.24k forks source link

"Equivalent to" code for zip is wrong in Python 3 #54238

Closed bef3b989-2e47-4e9c-91d3-504408b50b75 closed 14 years ago

bef3b989-2e47-4e9c-91d3-504408b50b75 commented 14 years ago
BPO 10029
Nosy @rhettinger, @abalkin, @orsenthil, @pkch

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: ```python assignee = 'https://github.com/rhettinger' closed_at = created_at = labels = ['type-bug', 'docs'] title = '"Equivalent to" code for zip is wrong in Python 3' updated_at = user = 'https://github.com/pkch' ``` bugs.python.org fields: ```python activity = actor = 'rhettinger' assignee = 'rhettinger' closed = True closed_date = closer = 'rhettinger' components = ['Documentation'] creation = creator = 'max' dependencies = [] files = [] hgrepos = [] issue_num = 10029 keywords = [] message_count = 14.0 messages = ['118025', '118026', '118027', '118028', '118115', '118116', '118122', '118129', '118211', '118219', '118300', '118306', '118309', '118314'] nosy_count = 7.0 nosy_names = ['rhettinger', 'belopolsky', 'orsenthil', 'stutzbach', 'docs@python', 'Douglas.Leeder', 'max'] pr_nums = [] priority = 'normal' resolution = 'fixed' stage = 'needs patch' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue10029' versions = ['Python 3.1', 'Python 3.2'] ```

bef3b989-2e47-4e9c-91d3-504408b50b75 commented 14 years ago

The sample code explaining zip function is incorrect at http://docs.python.org/py3k/library/functions.html?highlight=zip#zip:

def zip(*iterables):
    # zip('ABCD', 'xy') --> Ax By
    iterables = map(iter, iterables)
    while iterables:
        yield tuple(map(next, iterables))

See http://stackoverflow.com/questions/3865640/understanding-zip-function for discussion.

abalkin commented 14 years ago

The relevant comment at Stack Overflow is:

""" It looks like it's a bug in the documentation. The 'equivalent' code working in python2, but not in python3, where it has an infinite loop.

And the latest version of the documentation has the same problem: http://docs.python.org/release/3.1.2/library/functions.html

Look like change r61361 was the problem, as it merged changes from python 2.6 without verifying that they were correct for python 3. """

cfc9f3e0-e33f-4ecd-9ddd-4123842d6c1e commented 14 years ago

The code was taken from the itertools.izip documentation for Python 2, where it did work.

In Python 2, next() raises StopIteration which is propagated up and causes the izip() to stop. In Python 3, map is itself a generator and the StopIteration terminates the map operation instead of terminating the zip operation.

abalkin commented 14 years ago

Note that the following variant where maps are replaced with list comprehensions seems to work:

def zip(*iterables):
    # zip('ABCD', 'xy') --> Ax By                                                                                                                                                      
    iterables = [iter(i) for i in iterables]
    while iterables:
    yield tuple([next(j) for j in iterables])
abalkin commented 14 years ago

As Daniel pointed out, the "equivalent to" code in builtins section comes from 2.x itertools documentation where and equivalent generator definition is presented for each function. While these definitions are helpful when used for documenting a module oriented towards more advanced users, I doubt that exposing novices who are looking up builtins to the yield keyword and generators is a good idea. The zip() example is particularly problematic. Conceptually, zip is a very simple function, but the "equivalent to" code is not easy to decipher. The reliance on StopIteration exception escaping from map to break out of the infinite loop is clever, but not obvious. Moreover, as this bug demonstrates, this trick relies on subtle details that changed in 3.x.

I suggest removing the "equivalent to" code from the zip section and replacing it with an example showing how to use zip with a for loop similar to the example illustrating enumerate.

cfc9f3e0-e33f-4ecd-9ddd-4123842d6c1e commented 14 years ago

I suggest removing the "equivalent to" code from the zip section and replacing it with an example showing how to use zip with a for loop similar to the example illustrating enumerate.

+1

bef3b989-2e47-4e9c-91d3-504408b50b75 commented 14 years ago

Personally, I find it impossible in some cases to understand exactly what a function does just from reading a textual description. In those cases, I always refer to the equivalent code if it's given. In fact that's the reason I was looking going the zip equivalent function!

I would feel it's a loss if equivalent code disappear from the docs.

I understand sometimes the code requires maintenance, but I'd rather live with some temporary bugs than lose the equivalent code.

As to subtleties of how it works, that's not really a concern, if that's the only way to understand the precise meaning of whatever it explains.

rhettinger commented 14 years ago

I'll update the docs with an equivalent that works and that has a comment showing when the StopIteration is raised and caught.

abalkin commented 14 years ago

On Thu, Oct 7, 2010 at 3:37 PM, Raymond Hettinger \report@bugs.python.org\ wrote: ..

I'll update the docs with an equivalent that works and that has a comment showing when the StopIteration is raised and caught.

In this case, I wonder if "equivalent to" code should be added to the section for enumerate() and map(). Also since any() and all() have "equivalent to" code, I think min(), max() and sum() deserve it as well.

rhettinger commented 14 years ago

Refuse the temptation to hypergeneralize ;-)

Also refuse the temptation to double the size of the docs (more != better).

In the case of min/max, the pure python versions may add some value in showing that the first match is what is returned. But the code will also be a bit convoluted because it needs paths for key-argument case and for the screwy interpretation of 1 arg vs multiple args.

The pure python code is there for any() and all() to show the early out feature and to suggest how you could roll-your-own if you want different behavior (such as returning the value of the first exception).

orsenthil commented 14 years ago

On Fri, Oct 08, 2010 at 06:25:26PM +0000, Alexander Belopolsky wrote:

In this case, I wonder if "equivalent to" code should be added to the section for enumerate() and map(). Also since any() and all() have "equivalent to" code, I think min(), max() and sum() deserve it as well.

I think, you are asking for consistency in docs, right?

As a sidenote those explanations should be okay, but merging it with the explanation of the functions may add to confusion (like the zip example did in this case).

Even I am +1 on removing that complex equivalent code in the zip documentation, as it can confusing to the newcomer. But I am also okay with moving that 'equivalent to code' further down in the explanation.

rhettinger commented 14 years ago

Max, thanks for reporting this. I've replaced the sample code, making it work correctly and more clearly showing the logic.

See r85345 and r85346.

Daniel, we need to sync-up on the meaning of marking a report as "accepted". Traditionally it denotes an approved patch, not a agreement that the bug is valid.

cfc9f3e0-e33f-4ecd-9ddd-4123842d6c1e commented 14 years ago

Daniel, we need to sync-up on the meaning of marking a report as "accepted". Traditionally it denotes an approved patch, not a agreement > that the bug is valid.

Woops! Thanks for the correction. For what it's worth, a quick search of issues with Resolution: accepted and Stage: needs patch suggests that Éric Araujo has perhaps been using the same misinterpretation as I have. I'm not sure if I got the behavior from him, if he got it from me, or if we both arrived there independently. ;-)

rhettinger commented 14 years ago

Please pass the word along if you get a chance :-)