| msg165476 - (view) |
Author: Chris Jerdonek (chris.jerdonek) *  |
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) * |
Date: 2012-07-26 11:00 |
| This is a very simple patch. |
|
|
| msg166689 - (view) |
Author: Chris Jerdonek (chris.jerdonek) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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)  |
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) * |
Date: 2012-08-18 00:52 |
| Confirmed that 2.7 raises the same errors (as I expected) using your test. Thanks, Chris. |
|
|