[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset cb1aafc9ad7e by Yury Selivanov in branch '3.5': Issue #24400: Resurrect inspect.isawaitable() https://hg.python.org/cpython/rev/cb1aafc9ad7e New changeset a14f6a70d013 by Yury Selivanov in branch 'default': Merge 3.5 (Issue #24400) https://hg.python.org/cpython/rev/a14f6a70d013 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: I think we should add inspect.isawaitable(), and have it pass for *anything* that can be used in an await expression (whether that's by implementing the Awaitable protocol, or due to special casing at the interpreter level). I agree. I've committed the change. Larry, please make sure that the latest patch lands in 3.5beta3. It's really important. -- nosy: +larry resolution: - fixed status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Ben Darnell added the comment: I don't think operator.getfuture() is possible because there are multiple ways of turning an awaitable into a Future. asyncio has one way; tornado has another. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: My hypothetical operator.getfuture() would be a functional spelling of what an await expression does to retrieve a future from an awaitable object. Whether that's actually useful is an open question, hence deferring the idea to 3.6 at the earliest :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: might be useful in the future is an API design red flag that suggests to me we may not know what good looks like here yet. At the same time, we need something Tornado et al can use to accept the can be used with await expressions, but doesn't implement the Awaitable protocol types. However, I think we can actually evaluate the consequences of two alternative designs, and decide based on the implications: a) Add inspect.isawaitable(obj), tell people to use that over checking just the ABC, since an interpreter may allow more types than just Awaitable instances. b) Add an inspect._islegacyawaitable(obj) API, and tell people to use isinstance(obj, Awaitable) or inspect._islegacyawaitable(obj), rather than just checking the ABC. I think it makes sense to document that there are types acceptable to the await expression that don't implement the Awaitable protocol, just as there are types acceptable to iter() that don't implement the Iterable protocol: class Seq: ... def __getitem__(self, idx): ... raise IndexError ... iter(Seq()) iterator object at 0x7fa6ac596748 import collections isinstance(Seq(), collections.Iterable) False In 3.6, we can add an API like operator.getfuture() to expose the implementation of the retrieve a future from an awaitable part of the await expression to Python so folks can switch to a try it and see if it works approach, rather than having to check with the inspect module. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: Sorry, I forgot to state my main conclusion in comparing the consequences of adding inspect.isawaitable vs adding an API specifically for checking isn't Awaitable, but can be used in an await expression: I think we should add inspect.isawaitable(), and have it pass for *anything* that can be used in an await expression (whether that's by implementing the Awaitable protocol, or due to special casing at the interpreter level). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: 1. abc.Coroutine and abc.Awaitable will guarantee that objects that implement them have '__await__' and it's safe to access it (that means that they will fail for generator-based coroutines). Absolutely. That was the main theme behind the whole type split. 2. inspect.isgenerator() can be used along with inspect.isawaitable() to detect generator-based coroutines (generators with CO_ITERABLE_COROUTINE flag). I still don't like the idea of having an inspect.isawaitable() that only checks for an ABC. If you want an ABC test, say it in your code. If you want to test for an __await__ attribute, say it. A helper function inspect.isawaitable() suggests that there is some (builtin) type to inspect here, not a protocol. ABCs are about abstract protocols, inspect is about concrete types. If you want to cover the iterable coroutine case, why not add an inspect helper function for that? That's clearly a concrete type (even more concrete than a concrete type) that can be inspected. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: If you want to cover the iterable coroutine case, why not add an inspect helper function for that? That's clearly a concrete type (even more concrete than a concrete type) that can be inspected. Because I view iterable coroutines as a temporary, transitional thing. 'inspect.isawaitable()' might be useful in the future even when we remove CO_ITERABLE_COROUTINE. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Guido, Ben, Stefan, Nick, I want to re-open this issue. I'm still not satisfied with the current state of things, mainly with the __instancecheck__ hack for Awaitable Coroutine ABCs (as Ben initially suggested). I think we should remove the __instancecheck__ from the ABCs, and resurrect inspect.isawaitable() function: 1. abc.Coroutine and abc.Awaitable will guarantee that objects that implement them have '__await__' and it's safe to access it (that means that they will fail for generator-based coroutines). 2. inspect.isgenerator() can be used along with inspect.isawaitable() to detect generator-based coroutines (generators with CO_ITERABLE_COROUTINE flag). -- resolution: fixed - status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset a9d38701536d by Yury Selivanov in branch '3.5': Issue #24400: Add one more unittest for CoroutineType.__await__ https://hg.python.org/cpython/rev/a9d38701536d -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset b2a3baa1c2b0 by Yury Selivanov in branch '3.5': Issue #24400: Mention that __instancecheck__ is used in abc.Awaitable and Coroutine https://hg.python.org/cpython/rev/b2a3baa1c2b0 New changeset 4bf1d332fe73 by Yury Selivanov in branch 'default': Merge 3.5 (issue #24400) https://hg.python.org/cpython/rev/4bf1d332fe73 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Martin Panter added the comment: The last change to /Doc/conf.py seems to have screwed up my docs build. Was that an accident? $ make -C Doc/ htmlsphinx-build -b html -d build/doctrees -D latex_paper_size= . build/html Running Sphinx v1.2.3 loading pickled environment... done Theme error: no theme named 'classic' found (missing theme.conf?) make: *** [build] Error 1 [Exit 2] -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset 68996acdec6f by Yury Selivanov in branch '3.5': docs/conf: Undo changes in b2a3baa1c2b0; issue #24400 https://hg.python.org/cpython/rev/68996acdec6f -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Thanks, Martin, it was indeed something that shouldn't been committed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Martin Panter added the comment: Opened Issue 24541 related to this latest change. The test and documentation are still inconsistent, even if the test passes. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
R. David Murray added the comment: Look like you forgot to adjust test_inspect for the removal. eg: http://buildbot.python.org/all/builders/AMD64%20Windows8.1%20Non-Debug%203.x/builds/54 -- nosy: +r.david.murray status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Yury Selivanov yseliva...@gmail.com: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Look like you forgot to adjust test_inspect for the removal. eg: My bad. Thanks, David! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset 0b7c313851ca by Yury Selivanov in branch '3.5': Issue #24400: Fix failing unittest https://hg.python.org/cpython/rev/0b7c313851ca New changeset 8c85291e86bf by Yury Selivanov in branch 'default': Merge 3.5 (Issue #24400) https://hg.python.org/cpython/rev/8c85291e86bf -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: isawaitable(), however, should continue using abc.Awaitable, since it only checks for __await__ presence on the type (or should we just drop it?) I'd really remove it. It's not referring to an actual type, so it doesn't fit the purpose of the inspect module. The fact that the only place where collections.abc is used in this module is in isawaitable() should be a clear indication that it's misplaced. +1. I added 'isawaitable()' before we decided to add ABCs, and now it is redundant (and we don't have isiterable, ishashable etc) Ben, I saw that you're using isawaitable() in tornado, but it looks like you can easily switch the code to use Awaitable ABC, right? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Ben Darnell added the comment: Yes, I can switch use the ABC instead, and I agree that it doesn't make sense to have the inspect method if it's going to be equivalent to the ABC. I'm happy with the outcome here but AFAIK the original issue still stands: the Awaitable ABC is unusual in that `isinstance(x, Awaitable)` may produce different results than `issubclass(x.__class__, Awaitable)`. I'd like to see more explicit documentation of the facts that A) functools.singledispatch is not always equivalent to isinstance() and B) Awaitable is one case (the only one in the stdlib?) where this distinction occurs. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset e20c197f19d6 by Yury Selivanov in branch '3.5': Issue #24400: Remove inspect.isawaitable(). https://hg.python.org/cpython/rev/e20c197f19d6 New changeset 800bf6a0e0d5 by Yury Selivanov in branch 'default': Merge 3.5 (Issue #24400) https://hg.python.org/cpython/rev/800bf6a0e0d5 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: isawaitable(), however, should continue using abc.Awaitable, since it only checks for __await__ presence on the type (or should we just drop it?) I'd really remove it. It's not referring to an actual type, so it doesn't fit the purpose of the inspect module. The fact that the only place where collections.abc is used in this module is in isawaitable() should be a clear indication that it's misplaced. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: I originally planned to make the next Cython release patch the Generator and Coroutine ABCs into collections.abc, but I now think it would be worth uploading an abc_backports package to PyPI instead that does that and on which asyncio, tornado and other packages could simply depend with a try-import. Done: https://pypi.python.org/pypi/backports_abc Feedback welcome. Stefan -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset 3a78be4bcbde by Yury Selivanov in branch '3.4': Issue #24400: Fix CoroWrapper for 'async def' coroutines https://hg.python.org/cpython/rev/3a78be4bcbde New changeset 338597d2e93b by Yury Selivanov in branch '3.5': Issue #24400: Fix CoroWrapper for 'async def' coroutines https://hg.python.org/cpython/rev/338597d2e93b New changeset 2e4d604c449d by Yury Selivanov in branch 'default': Issue #24400: Fix CoroWrapper for 'async def' coroutines https://hg.python.org/cpython/rev/2e4d604c449d -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset 8f4e738cb07f by Yury Selivanov in branch '3.5': Issue #24495, #24400: Test asyncio.Task.repr in debug mode https://hg.python.org/cpython/rev/8f4e738cb07f -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset f4b702672beb by Yury Selivanov in branch '3.4': asyncio: Merge changes from issue #24400. https://hg.python.org/cpython/rev/f4b702672beb -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset eb6fb8e2f995 by Yury Selivanov in branch '3.5': Issue #24325, #24400: Add more unittests for types.coroutine; tweak wrapper implementation. https://hg.python.org/cpython/rev/eb6fb8e2f995 New changeset 7a2a79362bbe by Yury Selivanov in branch 'default': Merge 3.5 (Issue #24325, #24400) https://hg.python.org/cpython/rev/7a2a79362bbe -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset 9aee273bf8b7 by Yury Selivanov in branch '3.5': Issue #24400, #24325: More tests for types._GeneratorWrapper https://hg.python.org/cpython/rev/9aee273bf8b7 New changeset fa097a336079 by Yury Selivanov in branch 'default': Merge 3.5 (issue #24325 #24400) https://hg.python.org/cpython/rev/fa097a336079 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Roundup Robot added the comment: New changeset 7a0a1a4ac639 by Yury Selivanov in branch '3.5': Issue #24400: Introduce a distinct type for 'async def' coroutines. https://hg.python.org/cpython/rev/7a0a1a4ac639 New changeset 44253ce374fc by Yury Selivanov in branch 'default': Issue #24400: Merge 3.5 https://hg.python.org/cpython/rev/44253ce374fc -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Nick, Martin, Stefan, thanks to all of you for the code review! -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: An updated patch is attached. I'll commit it tomorrow morning. -- Added file: http://bugs.python.org/file39764/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: I added three more comments to the review. The rest looks good to me, too. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: OK, let's go with cr_* prefix for coroutine type's slots. Nick, there is one more thing I'd be glad to receive your input on. Currently, inspect.iscoroutine(o) uses isinstance(o, abc.Coroutine) to do the check. Ben and Stefan reasonably ask to refactor isgenerator() to use abc.Generator. However, after giving this some thought, I think that inspect.iscoroutine() should actually use isinstance(types.CoroutineType). Lots of existing code assumes, that if isgenerator() returns True, then the object has gi_code etc, so the function returns True when the passed object is a pure Python generator. Same logic should work for iscoroutine(), right? isawaitable(), however, should continue using abc.Awaitable, since it only checks for __await__ presence on the type (or should we just drop it?) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: My view is that the gi_* prefixed attributes are the ducktyped representation of whether an object is a generator-iterator or a coroutine. With the change, it means hasattr(obj, 'cr_running') is to hasattr(obj, 'gi_running') as isinstance(obj, collections.abc.Coroutine) is to isinstance(obj, collections.abc.Generator). The main lesson I personally learned from our experience with PEP 342 is that generator-as-iterator and generator-as-coroutine are fundamentally different concepts, and that having them *look* similar on the surface turned out to be inherently confusing. I see PEP 492 as the natural consequence of that perspective: from an eval loop perspective, native coroutines and generators-as-coroutines remain essentially interchangeable, but on the surface, they'll now look *completely* different. That allows folks to learn them as two distinct concepts (generator-iterator and coroutine), and defer (perhaps indefinitely) the realisation that they share a common underlying foundation. Originally we were happy to let that distinction be purely syntactic, and still let the underlying implementation details leak through in the object model. This issue helped highlight that that approach was likely to prove to be a recipe for hard to debug problems as we tried to figure out where we were dealing with generator-iterator objects, native coroutine objects and generator-as-coroutine objects. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: Based on that last comment, I realised there's something else @types.coroutine should now do: delegate both the existing gi_* attributes *and* the new cr_* attributes to the underlying generator. That should allow all of the poking around in code object flags to be encapsulated entirely in the types module, as the 3 kinds of object would subsequently be distinguishable by their attributes: gi_running: generator-iterator or generator-as-coroutine cr_running: native coroutine or generator-as-coroutine both: generator-as-coroutine -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: I believe the key issue of concern for Cython is being able to emulate the native CPython types in a way that things like asyncio just work, rather than specifically needing to have inspect report them as native generators and coroutines. As such, having inspect.isgenerator() and inspect.iscoroutine() be consistent in checking specifically for the *native* types makes sense to me, but it should be made clear that they're stricter checks than isinstance(obj, collections.abc.Generator) and isinstance(obj, collections.abc.Coroutine). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Nick, Martin, here's an updated patch with all your comments addressed. -- Added file: http://bugs.python.org/file39751/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: Latest version looks good to me - I'd suggest we merge this, and file any remaining problems as separate issues. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Actually, I'm not sure that we should use 'cr_*' prefix instead of 'gi_*' for coroutines. Coroutines reusing generators machinery is a two-fold thing: on the one hand it makes the implementation simpler; on the other -- __await__ must return an *iterator*. If you want to push values into __await__, it must return a *generator*. Essentially, as Guido said in one of his emails, we should see PEP 492 as a refinement of 'yield from' and existing generator-based coroutines. I love the idea of separating types for coroutines and generators, but I'm not so sure about 'cr_*' prefix. Nick, Guido, what do you think about this? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Yury Selivanov yseliva...@gmail.com: Added file: http://bugs.python.org/file39745/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: New patch is attached. Updates: 1. Coroutine type now has 'cr_*' slots *both* in Python *and* in C. 2. set_coroutine_wrapper now works *only* on coroutines created by 'async def' functions (generators wrapped with types.coroutine won't be intercepted). 3. RuntimeWarning that coroutine wasn't ever awaited will be raised *only* for 'async def' coroutines. 4. Coroutine objects no longer have 'CO_GENERATOR' flag. -- Added file: http://bugs.python.org/file39740/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: In my last set of review comments, I suggested changing the Python level attributes for coroutine objects to cr_frame, cr_code, and cr_running. That reminded me that now that coroutines are their own type, we should also give them their own state introspection API, matching the API for generators: https://docs.python.org/3/library/inspect.html#current-state-of-a-generator -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Another iteration of the patch is attached. Nick, I think it's ready for your review. -- stage: needs patch - patch review type: - enhancement Added file: http://bugs.python.org/file39729/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Yury Selivanov yseliva...@gmail.com: Added file: http://bugs.python.org/file39730/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Yury Selivanov yseliva...@gmail.com: Added file: http://bugs.python.org/file39722/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Nick, Stefan, Martin, Please find an updated patch attached. All issues that you guys found (thanks a lot for the reviews btw!) should be resolved. -- Added file: http://bugs.python.org/file39720/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Martin Panter vadmium...@gmail.com: -- nosy: +vadmium ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Martin Panter added the comment: One problem with 2015-06-10’s patch (x86-64 Linux): async def f(): ... pass ... c = f() w = c.__await__() w coroutine_wrapper object at 0x7f1013130b88 dir(w) TypeError: object does not provide __dir__ type(w) Segmentation fault (core dumped) [Exit 139] Strangely, if I call w.__dir__() first, everything seems to start behaving properly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: Regarding the idea of doing a typedef for the new coro type at the C level: looking further at the way the new type integrates with the eval loop, it's essential that they actually retain the exact same memory layout if we don't want to rewrite a whole lot of code. I still like the idea of adding the typedef so we can at least make the distinction in cases where the C level handling *isn't* shared between the two types, it would just need a great big disclaimer saying that the memory layout of the coroutine struct can't be altered independently of the generator layout. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: I agree that a typedef is a good idea. It doesn't cost anything but gives us more freedom for future changes. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Guido van Rossum added the comment: FYI I am on vacation and don't have the bandwidth to look into this, so I hope you will all work together to find a solution without my help. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: Low level review sent. Regarding the new opcode, it appears the main thing it provides is early detection of yielding from a coroutine in a normal generator, which is never going to work properly (a for loop or other iterative construct can't drive a coroutine as it expects to be driven), but would be incredibly painful to debug if you did it accidentally. For me, that counts as a good enough reason to add a new opcode during the beta period (just as adding the new types is necessary to fix various problems with the original not-actually-a-different-type implementation). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: A couple of high level observations: 1. As Yury notes, more clearly separating coroutines and generators is the intent of the PEP. The computer could clearly cope with them being the same class, but humans tended to get confused. We were still blurring that line too much in the initial implementation, this patch aims to fix that by *really* introducing a separate class for native coroutines. 2. Missing integrations in other modules that are likely to result in misbehaviour in user code (such as the inspect module not accounting for the new ABCs) are good candidates for filing as behaviour bugs - finding that kind of gap in PEP implementations is one of the benefits of the beta period. 3. Integrating nicely with both asyncio and Tornado is a good pragmatic design goal to avoid making too many asyncio specific assumptions -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Ben Darnell added the comment: With the two changes I described things appear to be working, although I've only done light testing so far. For inspect.isgenerator(), my use case is here: https://github.com/tornadoweb/tornado/blob/2971e857104f8d02fa9107a0e13f50170eb4f30d/tornado/gen.py#L222 I currently do isinstance(x, types.GeneratorType), which will fail if x is actually a GeneratorWrapper. I can change this to use collections.abc.Generator when it exists, but then I'd have to have some conditional logic to switch between collections.abc and types depending on what version I'm running on. It would be nice if that were encapsulated in inspect.isgenerator(). More generally, the inconsistency between isgenerator() and iscoroutine() is kind of odd. I would expect that either all inspect functions or none of them would use a suitable ABC if one exists. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: I currently do isinstance(x, types.GeneratorType), which will fail if x is actually a GeneratorWrapper. I can change this to use collections.abc.Generator when it exists, but then I'd have to have some conditional logic to switch between collections.abc and types depending on what version I'm running on. I originally planned to make the next Cython release patch the Generator and Coroutine ABCs into collections.abc, but I now think it would be worth uploading an abc_backports package to PyPI instead that does that and on which asyncio, tornado and other packages could simply depend with a try-import. It would be nice if that were encapsulated in inspect.isgenerator(). +1, code that needs exactly a generator, e.g. for gi_running and friends, can still test for isinstance(obj, types.GeneratorType) in a completely backwards compatible way, which is more explicit anyway. More generally, the inconsistency between isgenerator() and iscoroutine() is kind of odd. I would expect that either all inspect functions or none of them would use a suitable ABC if one exists. Yes, it's odd. Either way would work (types is for types only vs. types includes protocols), but having both disagree seems wrong. I think the mere fact that there is a higher level function than isinstance() suggests that it should really be more high level and not just doing exactly the same for all times. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: With the two changes I described things appear to be working, although I've only done light testing so far. Glad to hear that! I've attached a new patch fixing types.coroutine per your request. More generally, the inconsistency between isgenerator() and iscoroutine() is kind of odd. I would expect that either all inspect functions or none of them would use a suitable ABC if one exists. iscoroutine() is just a newer API than isgenerator(). I'll create a new issue for this soon. -- Added file: http://bugs.python.org/file39675/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Yury Selivanov yseliva...@gmail.com: -- hgrepos: +312 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Cleaned up the patch a little bit. -- Added file: http://bugs.python.org/file39665/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Andrew Svetlov andrew.svet...@gmail.com: -- nosy: +asvetlov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: One more patch fixing minor bug in types.coroutine + a unittest for that. The patch should be ready for reviews. -- Added file: http://bugs.python.org/file39666/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Stefan Behnel added the comment: I added some review comments. The main thing is that the coroutine type should be awaitable. For reference, here's my current implementation for Cython. It's a bit more involved as it needs to support Python 2.6+. I may also add some special casing for CPython's own coroutine type when compiling in Py3.5 if this change makes it in. https://github.com/cython/cython/blob/bb0dec2fab91cbde443e6756c3dc29ee009caba7/Cython/Utility/Coroutine.c -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Nick Coghlan added the comment: A quick scan of which files have been modified suggests the new opcode still needs docs (although I think the PEP 492 docs in general are still pending, in which case, this could just be rolled into that) I'll review the full patch later today (too big to review on my phone) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Nick Coghlan added the comment: A quick scan of which files have been modified suggests the new opcode still needs docs (although I think the PEP 492 docs in general are still pending, in which case, this could just be rolled into that) Sure. If the patch looks good I'll update it with the docs! -- nosy: +Yury.Selivanov ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: (although I think the PEP 492 docs in general are still pending, in which case, this could just be rolled into that) Actually most of pep 492 docs are merged already (including new opcodes) via issue24180. They can be definitely improved though (I'll try to reserve some time just for that closer to the rc) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Please find attached a new patch. Stefan, while working on the patch, I (re-)discovered that __await__ for coroutines should return an iterator that also implements '.send', '.throw', and '.close', to comply with PEP 380 yield from implementation: https://www.python.org/dev/peps/pep-0380/#proposal Please try to compile this python file: https://gist.github.com/1st1/4ee1d072309068dd2798 -- Added file: http://bugs.python.org/file39669/corotype.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: Nick, Guido, I think we should commit this. While working on this I'm getting more and more confident that it's the right thing to do. I like that coroutines implement __await__ in the latest patch -- PEP 492 just now clicks with PEP 380. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Ben Darnell added the comment: 4. While this patch addresses initial request from Ben only partially (generator-based coroutines still require __instancecheck__), A partial solution doesn't mean much to me: as long as the __instancecheck__ is sometimes necessary, I'll have to use inspect.iscoroutine all the time instead of using singledispatch with Awaitable. If anything, this just magnifies the risk of mysterious failures as things will work with async def but not yield from. I think I'd rather not have the ABC than have one with this kind of quirk. All this checking for coroutine-ness feels very strange to me. It's anti-duck-typing: all generators have all the methods necessary to satisfy the coroutine interface, but you can't use them as coroutines without some magical indication that that's what you meant. Prior to 3.5, a coroutine was just a callable returning a generator. Now, not only must it return a generator with the special coroutine flag, the callable itself must be of the right type, which causes problems when the underlying generator function is wrapped in complex ways (https://github.com/tornadoweb/tornado/blob/2971e857104f8d02fa9107a0e13f50170eb4f30d/tornado/testing.py#L476). Attempting to divide generators into awaitable and non-awaitable subsets is a complex solution to a problem that I'm not convinced is real. Was there a problem in practice with Python 3.4's asyncio in which people used yield from in a coroutine with generators that were intended to be iterators instead? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: All this checking for coroutine-ness feels very strange to me. It's anti-duck-typing: [..] Why is it anti-duck-typing? Awaitable is an object that implements __await__. With this patch coroutines are a separate type with __await__ (although, ceval doesn't use it to make things faster). In asyncio we check for coroutine-ness only to raise errors if someone passes a wrong object, or to make @asyncio.coroutine work (which will go away eventually). Now, not only must it return a generator with the special coroutine flag, the callable itself must be of the right type [..] Not sure what you mean here. It doesn't matter what callable is. It only matters if it returns a native coroutine, a generator-based coroutine, or an object with __await__. Attempting to divide generators into awaitable and non-awaitable subsets is a complex solution to a problem that I'm not convinced is real. Well, 'await' expression is a new operator, and it makes total sense to limit its usage only to awaitables. Awaiting on a generator that yields a fibonacci sequence just doesn't make any sense, and *is* error prone. I think it would be a major mistake to allow this just to make things a little bit more convenient during the transition period. This particular patch does not divide generators in awaitables and non-awaitables, it introduces a new type for 'async def' coroutines. All confusion with old generator-based coroutines and @coroutine decorator is here only because we try to be backwards compatible; the compatibility layer can be removed in 3.7 or 3.8. Was there a problem in practice with Python 3.4's asyncio in which people used yield from in a coroutine with generators that were intended to be iterators instead? Yes, a lot of people were confused where they have coroutines and where they have generators, and this was even mentioned in the Rationale section of the PEP. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: On June 9, 2015 at 11:11:11 PM, Ben Darnell (rep...@bugs.python.org) wrote: [..] The type of the callable matters for the types.coroutine decorator. In order to get a coroutine object instead of a generator object, I must apply types.coroutine to the actual underlying generator, and not any wrapper. I get it now, thanks for explaining. FWIW, I’ve recently updated types.coroutine: https://github.com/python/cpython/blob/eae2bd7c0718b8e4333bd16134e37810e32c050c/Lib/types.py#L164 Instead of unwrapping the callable, it wraps its result with a special object with __await__. For instance, this enables regular functions (not generator functions) that are decorated with @types.coruoutine and return a generator, or a generator-like object compiled with Cython to work with ‘await’. Maybe this is something you can use in Tornado? There is also issue24325 — to implement most of types.coroutine in C to make it faster. I understand that yield-based coroutines can be confusing, but is this particular kind of confusion bad enough that if someone writes 'await fibonacci_generator()' we have to raise TypeError: object generator can't be used in 'await' expression instead of RuntimeError: Task got bad yield: 1? (that error message from asyncio could definitely be improved, but the fact that no one has improved it suggests that the problem isn't that severe) My goal here is to make it possible for Tornado applications running on 3.5 to use 'async def' and 'await' even though Tornado itself must remain compatible with 2.7 (as do many of the libraries that make up the Tornado ecosystem; I don't want libraries to have to update for compatibility with this feature). This is doable (I think) but non-trivial in the current design; making all generators awaitable would make it easier. I understand. My line of thoughts here is: we are introducing a new operator. However we define it now, will be set in stone after 3.5 is released. We should try to solve possible backwards compatibility issues without making its semantics less strict just to make things easier. Please take a look at the recently refactored types.coroutine (there is a link above). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Ben Darnell added the comment: GeneratorWrapper helps, but it fails when applied to non-generator functions that return a value (while both tornado.gen.coroutine and asyncio.coroutine take pains to support such usage). The raise TypeError should be removed; just return the result without wrapping if it's not a generator. GeneratorWrapper also runs afoul of some places where I do explicit type checking instead of duck typing (isinstance(x, types.GeneratorType)). Using the Generator ABC doesn't work because the generator methods are set as attributes on __init__ instead of defined on the class. The methods should be defined on the class, even if they're overwritten with instance attributes later for speed. (related: inspect.iscoroutine is defined in terms of collections.abc.Coroutine. Should inspect.isgenerator be redefined to use the new collections.abc.Generator?) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Ben Darnell added the comment: On Tue, Jun 9, 2015 at 10:12 PM, Yury Selivanov rep...@bugs.python.org wrote: Yury Selivanov added the comment: All this checking for coroutine-ness feels very strange to me. It's anti-duck-typing: [..] Why is it anti-duck-typing? Awaitable is an object that implements __await__. With this patch coroutines are a separate type with __await__ (although, ceval doesn't use it to make things faster). Anti-duck-typing isn't quite the right word. What I meant was that Python 3.4 had generators that were used as coroutines. When we introduced __await__ in 3.5, we could have added __await__ as a method for all generators, since all generators have the necessary send/throw/close methods. But we didn't, and now we require an explicit marker that a given object was intended for a particular use instead of inferring that intention from the set of methods available. In asyncio we check for coroutine-ness only to raise errors if someone passes a wrong object, or to make @asyncio.coroutine work (which will go away eventually). The check for coroutine-ness is not just in asyncio; it happens in the core interpreter. Tornado has to adapt to this check in order to interoperate with 'async def' coroutines. Now, not only must it return a generator with the special coroutine flag, the callable itself must be of the right type [..] Not sure what you mean here. It doesn't matter what callable is. It only matters if it returns a native coroutine, a generator-based coroutine, or an object with __await__. The type of the callable matters for the types.coroutine decorator. In order to get a coroutine object instead of a generator object, I must apply types.coroutine to the actual underlying generator, and not any wrapper. Attempting to divide generators into awaitable and non-awaitable subsets is a complex solution to a problem that I'm not convinced is real. Well, 'await' expression is a new operator, and it makes total sense to limit its usage only to awaitables. Awaiting on a generator that yields a fibonacci sequence just doesn't make any sense, and *is* error prone. I think it would be a major mistake to allow this just to make things a little bit more convenient during the transition period. This particular patch does not divide generators in awaitables and non-awaitables, it introduces a new type for 'async def' coroutines. All confusion with old generator-based coroutines and @coroutine decorator is here only because we try to be backwards compatible; the compatibility layer can be removed in 3.7 or 3.8. There are three eras of coroutines to consider: 'async def' in 3.5+, 'yield from' in 3.3-3.4, and 'yield' in 2.5+. I'm trying to provide a path from 'yield' to 'async def'; this compatibility layer will remain relevant as long as people are migrating from 2.7. Was there a problem in practice with Python 3.4's asyncio in which people used yield from in a coroutine with generators that were intended to be iterators instead? Yes, a lot of people were confused where they have coroutines and where they have generators, and this was even mentioned in the Rationale section of the PEP. I understand that yield-based coroutines can be confusing, but is this particular kind of confusion bad enough that if someone writes 'await fibonacci_generator()' we have to raise TypeError: object generator can't be used in 'await' expression instead of RuntimeError: Task got bad yield: 1? (that error message from asyncio could definitely be improved, but the fact that no one has improved it suggests that the problem isn't that severe) My goal here is to make it possible for Tornado applications running on 3.5 to use 'async def' and 'await' even though Tornado itself must remain compatible with 2.7 (as do many of the libraries that make up the Tornado ecosystem; I don't want libraries to have to update for compatibility with this feature). This is doable (I think) but non-trivial in the current design; making all generators awaitable would make it easier. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: GeneratorWrapper helps, but it fails when applied to non-generator functions that return a value (while both tornado.gen.coroutine and asyncio.coroutine take pains to support such usage). The raise TypeError should be removed; just return the result without wrapping if it's not a generator. I think this is reasonable. I'll try it tomorrow to see if there are any corner cases, but I doubt there are any. GeneratorWrapper also runs afoul of some places where I do explicit type checking instead of duck typing (isinstance(x, types.GeneratorType)). Using the Generator ABC doesn't work because the generator methods are set as attributes on __init__ instead of defined on the class. The methods should be defined on the class, even if they're overwritten with instance attributes later for speed. Sure, this can be fixed too. Could you please update types.coroutine locally and verify that it will work for Tornado? (related: inspect.iscoroutine is defined in terms of collections.abc.Coroutine. Should inspect.isgenerator be redefined to use the new collections.abc.Generator?) Since abc.Generator is a new thing I guess we still can do that. Do you have a good use case for it? We need a good one to convince Larry to include this post-beta. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Yury Selivanov added the comment: I think that the only proper way to solve this is to make coroutines a separate type. I've actually prototyped that before: https://github.com/1st1/cpython/commit/a3f1059590f496bf77b33edb023c8cdbc1d30798 -- assignee: - yselivanov components: +Interpreter Core nosy: +ncoghlan versions: +Python 3.6 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Guido van Rossum added the comment: I think it's actually good feedback and we should fix this during the beta. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
Changes by Ned Deily n...@acm.org: -- priority: normal - release blocker stage: - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24400] Awaitable ABC incompatible with functools.singledispatch
New submission from Ben Darnell: The new collections.abc.Awaitable ABC relies on __instancecheck__, which makes it incompatible with functools.singledispatch (singledispatch works based on args[0].__class__; any instance-level information is discarded). This surprised me because the first thing I tried to do with Awaitable was add it to my singledispatch-based coroutine compatibility layer. Ideally coroutine would be an actual subclass of generator, instead of a generator with an extra bit set on the instance that changes how it answers isinstance() checks. That would be a big change, though, so it might be better to just document that Awaitable is kind of unusual (if we weren't already in the beta period I might argue that the ABCs should be removed and we should just use the functions in the inspect module instead). -- components: asyncio messages: 244942 nosy: Ben.Darnell, gvanrossum, haypo, yselivanov priority: normal severity: normal status: open title: Awaitable ABC incompatible with functools.singledispatch versions: Python 3.5 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24400 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com