[issue24439] Feedback for awaitable coroutine documentation
New submission from Martin Panter: Today I tried playing with the new “async def” functionality in the interactive interpreter, based on reading the documentation. Here are some bits that surprised me, so could be the focus of further documentation. (Initial documentation added in Issue 24180.) 1. Confusion between coroutine and generator terms <coroutine object f at 0x7f9a5ae45200> AttributeError: 'generator' object has no attribute '__await__' <class 'generator'> TypeError: coroutine-objects do not support iteration TypeError: can't send non-None value to a just-started generator Is it a coroutine object, or a generator object? The error messages etc seem to be split between the two terms. I guess the underlying implementation is closely shared between the two types, so old error messages still say “generator”, while new ones say “coroutine”. So there may not be an easy fix, but it was initially disconcerting. Perhaps a statement somewhere explaining coroutine instances are (based on / modified / a kind of / evolved from) generator instance objects, as appropriate. 2. Properties of a coroutine instance object <https://docs.python.org/dev/reference/datamodel.html#awaitable-objects> suggests that a coroutine instance should implement __await__(), but it apparently does not. How to drive through the coroutine’s await points? This should be documented, and linked from relevant places in the documentation; perhaps the “coroutine” glossary entry, “async” statement, and “await” expression. The best clue that I found in the current documentation was <https://docs.python.org/dev/library/collections.abc.html#collections.abc.Cor...>, suggesting to use send(). Without having studied the PEP recently, I was expecting __await__() to be implemented, and invoking it would be similar to calling next() on an old-skool generator-iterator. Instead I see that calling __await__() is semantically more like __iter__(): it is only invoked once per “await” expression, and the return value is iterated over. 3. Allergic reaction to “await” on the same line as “async def”
async def f(): await A(()) SyntaxError: invalid syntax
I see this is mentioned in the PEP that this is necessary so that “await” doesn’t have to be made a strict language keyword. But it should also be in the documentation. 4. Coroutine instances don’t implement the Coroutine ABC
Reading closely, the documentation says “coroutine compatible classes”, but given the liberal use of “coroutine” everywhere, I think there should be a more obvious warning of how much a coroutine is a Coroutine. ---------- assignee: docs@python components: Documentation messages: 245256 nosy: docs@python, vadmium, yselivanov priority: normal severity: normal status: open title: Feedback for awaitable coroutine documentation type: enhancement versions: Python 3.5, Python 3.6 _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Martin Panter added the comment: It seems that Issue 24400 may implement __await__() for native coroutine instances, making points 1, 2 and 4 mainly redundant. This would also bypass a fifth problem: the need for the mandatory yet largely useless send(None) argument. I am posting async-doc.patch, with these changes: * Distinguish between PEP 492’s “native coroutines” and other coroutines such as those already supported by asyncio and PEP 342 (“yield” expression and generator cleanup) * Move “coroutine” before “coroutine function” in the glossary. * Add links to “coroutine” glossary * Point 3: Explain about “async” and “await” becoming reserved keywords after a “def” header line * Part of point 2: List native coroutine instance methods and hint at relationship with generator iterator instances Still to do: how to drive an awaitable coroutine. Currently it seems you have to call coro.send(None), but if the current patch for Issue 24400 were applied, I think it would become next(coro), so I will leave this for later. ---------- dependencies: +Awaitable ABC incompatible with functools.singledispatch keywords: +patch stage: -> patch review Added file: http://bugs.python.org/file39714/async-doc.patch _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Yury Selivanov added the comment: Martin, thanks a lot for the feedback and patch! I'll review the patch when issue24400 lands. ---------- assignee: docs@python -> yselivanov nosy: +ncoghlan _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Yury Selivanov added the comment: Martin, if you want to help with the documentation, it would be great if you can help me with updating asyncio coroutines section: https://docs.python.org/3.5/library/asyncio-task.html#coroutines ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Martin Panter added the comment: When I update my patch I can try updating the asyncio section. However I have only had limited experience with asyncio, so feel free to suggest things to add. Here is a list of things I think may need changing: * “async def” routines are allowed in addition to generators in asyncio (e.g. in Task constructor) * Other awaitables are also accepted as asyncio coroutines * List of “yield from” actions could be augmented with “await”, “async for”, etc * The coroutines provided by asyncio are both iterable and awaitable, so they may be used with both “yield from” and “await” * Change references of asyncio.async() to ensure_future() ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Yury Selivanov added the comment: Hi Martin, I've left you some feedback in the code review.
* “async def” routines are allowed in addition to generators in asyncio (e.g. in Task constructor)
Right. I think we need to add some code samples too.
* Other awaitables are also accepted as asyncio coroutines
Depending on where, I guess. asyncio.Task should only accept when asyncio.iscoroutine is true -- abc.Coroutine, types.GeneratorType.
* List of “yield from” actions could be augmented with “await”, “async for”, etc
Not sure what you mean here.
* The coroutines provided by asyncio are both iterable and awaitable, so they may be used with both “yield from” and “await”
Right. We also need to make sure that it's documented that in order to have "yield from native_coro()", you have to decorate the gen function with 'asyncio.coroutine'. And we should mention that if you're targeting Python 3.5+ you should use the new syntax.
* Change references of asyncio.async() to ensure_future()
This is already done ;) Thanks! ---------- nosy: +asvetlov, gvanrossum, haypo _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Martin Panter added the comment: Here is a patch with more improvements to the coroutine documentation, including: * Remove expansion of “coroutine” glossary to include generators; it was not my original intention of this bug. Perhaps this can be dealt with separately in Issue 24087 if other people agree there is a problem. * Add Native Coroutine Objects section to /Doc/reference/datamodel.rst describing each method * Tweak coroutine and wrapper object doc strings to avoid generator terms * Mention native coroutines and “await” in the asyncio Coroutines section, where the existing text only mentions generators and “yield from” * Also recommend native coroutines, @asyncio.coroutine for calling native coroutines, etc I changed two of the asyncio coroutine examples to use “async def” rather than generators. There is a third example that still uses generators which I did not change, because I would also have to update the sequence diagram. ---------- Added file: http://bugs.python.org/file39788/async-doc.v2.patch _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Martin Panter added the comment: Thanks for reviewing this Yury. Here is a new patch: * Drop the “native” term; distinguish by referring to “async def” where necessary * Add generator version of display_date() coroutine example * Change chained coroutine example over to “async def” and updated sequence diagram * Change one other example over (then I ran into Issue 24495) * Include Yury’s new documentation for @types.coroutine from Issue 24325 ---------- Added file: http://bugs.python.org/file39798/async-doc.v3.patch _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Changes by Martin Panter <vadmium+py@gmail.com>: Added file: http://bugs.python.org/file39799/tulip_coro.png _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Roundup Robot added the comment: New changeset e31aad001fdb by Yury Selivanov in branch '3.5': Issue #24439: Improve PEP 492 related docs. https://hg.python.org/cpython/rev/e31aad001fdb New changeset d99770da3b2a by Yury Selivanov in branch 'default': Merge 3.5 (issue #24439) https://hg.python.org/cpython/rev/d99770da3b2a ---------- nosy: +python-dev _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Yury Selivanov added the comment: Committed. Thanks a lot, Martin! ---------- resolution: -> fixed stage: patch review -> resolved status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Martin Panter added the comment: Hi Yury. It looks like you missed my updated /Doc/library/tulip_coro.dia file. Here it is separately, in case you are having trouble extracting it from the patch. ---------- Added file: http://bugs.python.org/file39810/tulip_coro.dia _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Roundup Robot added the comment: New changeset e9c34e16f445 by Yury Selivanov in branch '3.5': Issue #24439: Update tulip_coro.dia https://hg.python.org/cpython/rev/e9c34e16f445 New changeset 15b8a62da6ff by Yury Selivanov in branch 'default': Merge 3.5 (Issue #24439) https://hg.python.org/cpython/rev/15b8a62da6ff ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
Yury Selivanov added the comment: Martin, it's now updated. (As for why it wasn't applied in the first place, I guess standard MacOS `patch` command fails to apply git binary diffs) ---------- _______________________________________ Python tracker <report@bugs.python.org> <http://bugs.python.org/issue24439> _______________________________________
participants (3)
-
Martin Panter -
Roundup Robot
-
Yury Selivanov