Repository navigation
add a Generator ABC #68206
Description
| BPO | 24018 |
|---|---|
| Nosy | @gvanrossum, @rhettinger, @pitrou, @scoder, @vstinner, @ambv, @vadmium, @1st1 |
| Files |
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:
assignee = 'https://github.com/rhettinger'
closed_at = <Date 2015-05-09.05:08:15.882>
created_at = <Date 2015-04-20.19:36:25.857>
labels = ['type-feature', 'library']
title = 'add a Generator ABC'
updated_at = <Date 2015-05-09.18:38:11.022>
user = 'https://github.com/scoder'bugs.python.org fields:
activity = <Date 2015-05-09.18:38:11.022>
actor = 'gvanrossum'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2015-05-09.05:08:15.882>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2015-04-20.19:36:25.857>
creator = 'scoder'
dependencies = []
files = ['39151', '39156', '39157', '39180', '39213']
hgrepos = []
issue_num = 24018
keywords = ['patch']
message_count = 36.0
messages = ['241675', '241677', '241681', '241682', '241683', '241692', '241700', '241701', '241702', '241703', '241704', '241705', '241716', '241717', '241723', '241725', '241736', '241846', '241956', '241993', '242054', '242071', '242072', '242077', '242087', '242094', '242102', '242104', '242445', '242612', '242794', '242796', '242797', '242798', '242826', '242827']
nosy_count = 10.0
nosy_names = ['gvanrossum', 'rhettinger', 'pitrou', 'scoder', 'vstinner', 'lukasz.langa', 'python-dev', 'Ludovic.Gasc', 'martin.panter', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue24018'
versions = ['Python 3.5']
Activity
scoder commentedon Apr 20, 2015
Currently, CPython tends to assume that generators are always implemented by a Python function that uses the "yield" keyword. However, it is absolutely possible to implement generators as a protocol by adding send(), throw() and close() methods to an iterator.
The problem is that these will usually be rejected by code that needs to distinguish generators from other input, e.g. in asyncio, as this code will commonly do a type check against types.GeneratorType. Instead, it should check for the expected protocol. The best way to achieve this is to extend the existing ABCs with a Generator ABC that external generator implementations can register on.
Related to bpo-24004 (asyncio). Asyncio could provide a backport copy of this for older Python versions.
I assume this is considered a new feature that cannot be added to 3.4 anymore?
gvanrossum commentedon Apr 20, 2015
It's missing tests. :-)
Otherwise looks quite sensible.
Also, shouldn't you override __subclasshook__ so you don't inherit it from Iterator?
scoder commentedon Apr 20, 2015
Ok, sure. Here's a new patch that adds tests and docs.
scoder commentedon Apr 20, 2015
Sorry, here's another doc fix.
pitrou commentedon Apr 20, 2015
Is it a problem that the check can't be done in a fast way from C code?
Other than that, sounds good to me.
rhettinger commentedon Apr 21, 2015
For the other ABCs, if you define the required abstract methods, you get working versions of all the mixin methods.
In the case of the Generator ABC, throw() and close() are useless empty stub methods. In the other ABCs, we leave optional methods out entirely. A user should expect that if isinstance(g, Generator) is true that all of the ABC methods will work.
Also, the return StopIteration in throw() doesn't seem correct. Shouldn't it raise the exception that was thrown?
>>> def f(): yield xTraceback (most recent call last): File "<pyshell#11>", line 1, in <module> g.throw(KeyError) File "<pyshell#9>", line 1, in f def f(): KeyErrorIdeally, there should be a practical example of where this ABC would be useful with anything other than a real generator.
scoder commentedon Apr 21, 2015
Here's a new patch that addresses the review comments. I kept throw() and close() non-abstract and added an example to the tests instead that implements a minimal generator by inheriting these methods from the Generator base class, using it as a mixin. It only needs to implement send().
I don't think it's unreasonable to assume that there are use cases where a generator that is implemented in a class instead of a yield-function does not require special cleanup actions in its close()/throw(). Or is it *required* that a generator stops working when close() is called?
There are also iterators that raise StopIteration at some point to signal that they are temporarily exhausted, but then eventually restart returning values from their __next__() method when they have received more to return. Avoids recreating the iterator object after each exhaustion cycle.
I extended the default implementation of close() to call throw(GeneratorExit), though, because that's how the protocol is defined. Unless users implement throw(), it won't make a difference for them. And if they do, they may even get away with not reimplementing close().
scoder commentedon Apr 21, 2015
C code can still quickly special case the generator type in the positive case, and it will usually be interested in an exact type match anyway. Determining that an object is *not* (compatible with) a generator might lead to some degradation, but in most cases, at least in the interpreter core, this would be an error case anyway, so being slower here should rarely hurt.
scoder commentedon Apr 21, 2015
Next question: should inspect.isgenerator() be changed? Or should its usages be replaced by isinstance(obj, Generator) ?
vadmium commentedon Apr 21, 2015
I agree that there is no big reason why we should force generators to stop working after close(). Your new default implementation of close() is probably the right thing too. I added a few new comments on Reitveld.
scoder commentedon Apr 21, 2015
Good catch with the RuntimeError. Patch updated.
scoder commentedon Apr 21, 2015
Replying to myself, one thing that speaks against this approach is that "inspect" also has the functions "getgeneratorlocals()" and "getgeneratorstate()", which depend on "gen.gi_frame". Cython could emulate that, but normal user code usually can't. It's definitely not part of the Generator protocol but an implementation detail of GeneratorType.
17 remaining items
rhettinger commentedon Apr 27, 2015
The latest patch looks good overall.
Łukasz, assigning back to you.
gvanrossum commentedon May 3, 2015
Yeah, looks good -- Łuke, can you commit this?
ambv commentedon May 5, 2015
Yup, will do.
scoder commentedon May 9, 2015
This is blocking bpo-24017 (async/await syntax).
gvanrossum commentedon May 9, 2015
Ask Yury if he'll commit it for you. It's ready.
python-dev commentedon May 9, 2015
New changeset ba5d7041e2f5 by Raymond Hettinger in branch 'default':
Issue bpo-24018: Add a collections.Generator abstract base class.
https://hg.python.org/cpython/rev/ba5d7041e2f5
scoder commentedon May 9, 2015
Thanks! Minor grouch: it should say "collections.*abc*.Generator" in the NEWS entry.
python-dev commentedon May 9, 2015
New changeset f7cc54086cd2 by Guido van Rossum in branch 'default':
Fix news entry for bpo-24018.
https://hg.python.org/cpython/rev/f7cc54086cd2
gvanrossum commentedon May 9, 2015
Fixed.
On Fri, May 8, 2015 at 10:28 PM, Stefan Behnel <[email protected]>
wrote: