classification
Title: generator docs should mention already-executing exception
Type: Stage: resolved
Components: Documentation Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: chris.jerdonek, docs@python, meador.inge, ncoghlan, python-dev, r.david.murray, terry.reedy
Priority: normal Keywords: easy, patch

Created on 2012-07-15 00:31 by chris.jerdonek, last changed 2012-08-18 00:52 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
issue-15355-1.patch chris.jerdonek, 2012-07-26 11:00 review
issue-15355-2.patch chris.jerdonek, 2012-07-29 03:54 review
Messages (12)
msg165476 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-15 00:30
I think the generator.__next__() documentation should say that it raises an exception if the generator is already executing:

http://docs.python.org/dev/reference/expressions.html#generator.__next__

I don't think this is currently mentioned anywhere in the section on yield expressions.

I think this is worth mentioning because this is different from the general situation for iterators, for example.  One consequence of this is that, unlike for iterators, using a bare generator in a multithreaded situation will always result in a critical section (since an iterator can be made to take care of its own locking, etc).
msg166472 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-26 11:00
This is a very simple patch.
msg166689 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-28 20:20
Terry, if when reviewing my patch for issue 15457, you also have time to review this much simpler patch (also in the documentation on generators), I would appreciate it.  If not, that's okay.
msg166704 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-07-29 02:26
Hmmm, in your original description you say that the 'generator.__next__' documentation should be changed, but in the patch the note applies to all generator methods.  From looking at the code it seems that the patch is correct and that '__next__', 'send', 'throw', and 'close' can all raise the "already executing" exception via 'gen_send_ex'.  Documenting this behavior seems reasonable, but you should probably mention what exception gets raises.

BTW, you don't need to make the Misc/NEWS changes a part of your patches.  A core dev will write that for you and since Misc/NEWS is changed so much it might conflict and make patches harder to apply across similar branches (say 3.2 and 3.3).
msg166711 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-29 03:54
> Hmmm, in your original description you say that the 'generator.__next__' documentation should be changed, but in the patch the note applies to all generator methods.

Thanks, Meador.  Yes, I realized that later.  Retitling now.  I will also add the exception type.  I wasn't sure if that was implementation-specific.

> BTW, you don't need to make the Misc/NEWS changes

Certainly -- will do from now on.  Thanks for telling me.  I had thought I was helping.  New patch attached.
msg166938 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-07-31 00:26
Meador, does this language look okay to you now that it states the exception type?
msg166956 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2012-07-31 03:30
Hi Chris, it seems reasonable to me, but I would feel more comfortable if someone (like Nick) that is more familiar with the generator implementation can comment on this as well.
msg167774 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-09 08:36
Nick, would you be able to take a look at this minor documentation patch re: generators?  Would you prefer this blanket statement, or an explicit (possibly repeated) statement inside the documentation of each method?
msg168430 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-08-17 03:18
Chris, if Nick is too busy to reply right now and you want to move this along, you could write some tests (not necessarily for inclusion in the test suite) to confirm that the doc you are adding is correct.  I don't know enough about generators to comment myself, I'd have to write tests :)
msg168478 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2012-08-17 21:01
Good suggestion, David.  Here is such sample test code.  It is adapted from the sample code for "ValueError: generator already executing" included in PEP 255:

def test_gen(call_gen_method):
    def gen():
        call_gen_method(me)
        yield 1
    me = gen()

    try:
        me.__next__()
    except Exception as e:
        print(repr(e))

test_gen(lambda g: g.__next__())
test_gen(lambda g: g.send(1))
test_gen(lambda g: g.throw(OSError))
test_gen(lambda g: g.close())

This outputs:

ValueError('generator already executing',)
ValueError('generator already executing',)
ValueError('generator already executing',)
ValueError('generator already executing',)
msg168488 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-08-18 00:50
New changeset dc4b00f51c48 by R David Murray in branch '3.2':
#15355: Mention already-executing Exception in generator docs.
http://hg.python.org/cpython/rev/dc4b00f51c48

New changeset 73f1ba3319dd by R David Murray in branch 'default':
Merge #15355: Mention already-executing Exception in generator docs.
http://hg.python.org/cpython/rev/73f1ba3319dd

New changeset a62309ae88a2 by R David Murray in branch '2.7':
#15355: Mention already-executing Exception in generator docs.
http://hg.python.org/cpython/rev/a62309ae88a2
msg168489 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-08-18 00:52
Confirmed that 2.7 raises the same errors (as I expected) using your test.

Thanks, Chris.
History
Date User Action Args
2012-08-18 00:52:13r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg168489

stage: patch review -> resolved
2012-08-18 00:50:44python-devsetnosy: + python-dev
messages: + msg168488
2012-08-17 21:01:53chris.jerdoneksetmessages: + msg168478
2012-08-17 03:18:21r.david.murraysetnosy: + r.david.murray
messages: + msg168430
2012-08-09 08:36:39chris.jerdoneksetmessages: + msg167774
versions: + Python 2.7, Python 3.2
2012-07-31 03:30:17meador.ingesetmessages: + msg166956
2012-07-31 00:26:13chris.jerdoneksetmessages: + msg166938
2012-07-29 03:54:56chris.jerdoneksetfiles: + issue-15355-2.patch

messages: + msg166711
title: generator.__next__() docs should mention exception if already executing -> generator docs should mention already-executing exception
2012-07-29 02:26:39meador.ingesetnosy: + meador.inge
messages: + msg166704
2012-07-28 20:20:21chris.jerdoneksetnosy: + terry.reedy
messages: + msg166689
2012-07-26 12:50:34ncoghlansetnosy: + ncoghlan
2012-07-26 11:39:28chris.jerdoneksetstage: patch review
2012-07-26 11:00:26chris.jerdoneksetfiles: + issue-15355-1.patch
keywords: + patch
messages: + msg166472
2012-07-15 00:31:00chris.jerdonekcreate