Awaitable ABC incompatible with functools.singledispatch

[bdarnell]

BPO	24400
Nosy	@gvanrossum, @ncoghlan, @scoder, @vstinner, @larryhastings, @bitdancer, @asvetlov, @bdarnell, @vadmium, @1st1
Files	corotype.patch corotype.patch corotype.patch corotype.patch corotype.patch corotype.patch corotype.patch: bug fixes & more tests for types.coroutine corotype.patch: Fix compiler warnings; micro optimizations corotype.patch: added documentation corotype.patch: update docstrings and error messages corotype.patch: cr_* slots for coroutine; changes to set_coro_wrapper; new funcs in inspect corotype.patch: fix typo; update whatsnew corotype.patch: docs updates; iscoroutine() no longer uses abc; other fixes per Nick's and Martin's review. corotype.patch: Fix few nits; enable ducktyping for inspect.getcoroutinelocals

^{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/1st1'
closed_at = <Date 2015-07-03.17:13:37.061>
created_at = <Date 2015-06-07.05:06:53.709>
labels = ['interpreter-core', 'type-feature', 'release-blocker', 'expert-asyncio']
title = 'Awaitable ABC incompatible with functools.singledispatch'
updated_at = <Date 2015-07-03.23:07:24.592>
user = 'https://github.com/bdarnell'

bugs.python.org fields:

activity = <Date 2015-07-03.23:07:24.592>
actor = 'ncoghlan'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2015-07-03.17:13:37.061>
closer = 'yselivanov'
components = ['Interpreter Core', 'asyncio']
creation = <Date 2015-06-07.05:06:53.709>
creator = 'Ben.Darnell'
dependencies = []
files = ['39664', '39665', '39666', '39669', '39675', '39720', '39722', '39723', '39729', '39730', '39740', '39745', '39751', '39764']
hgrepos = ['312']
issue_num = 24400
keywords = ['patch']
message_count = 74.0
messages = ['244942', '244961', '245005', '245013', '245014', '245073', '245074', '245076', '245077', '245083', '245097', '245098', '245100', '245101', '245102', '245103', '245105', '245108', '245109', '245113', '245114', '245125', '245129', '245130', '245136', '245179', '245210', '245300', '245304', '245307', '245414', '245440', '245475', '245490', '245507', '245529', '245549', '245552', '245553', '245557', '245570', '245579', '245585', '245615', '245636', '245637', '245742', '245746', '245749', '245757', '245762', '245776', '245962', '246001', '246002', '246004', '246008', '246009', '246010', '246013', '246039', '246040', '246050', '246051', '246052', '246137', '246140', '246141', '246150', '246152', '246191', '246192', '246229', '246230']
nosy_count = 12.0
nosy_names = ['gvanrossum', 'ncoghlan', 'scoder', 'vstinner', 'larry', 'r.david.murray', 'asvetlov', 'Yury.Selivanov', 'python-dev', 'Ben.Darnell', 'martin.panter', 'yselivanov']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue24400'
versions = ['Python 3.5', 'Python 3.6']

[bdarnell]

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).

[gvanrossum]

I think it's actually good feedback and we should fix this during the beta.

[1st1]

I think that the only proper way to solve this is to make coroutines a separate type. I've actually prototyped that before: 1st1@a3f1059

[scoder]

I agree that the "extra bit" is a quirk. Ideally, Coroutine and Generator would share a common base class that has "send", "throw" and "close". Additionally, a Coroutine would be an Awaitable whereas a Generator is an Iterator. The internal implementation of generators and coroutines would (or at least, could) then follow by splitting them into separate types.

The tricky bridge between the two is the types.coroutine() decorator, which is intended to say "this Generator is a Coroutine", i.e. it turns a Generator into an Awaitable Generator. As the current implementation shows, however, this can also be done without the internal flag. All this needs is a wrapper, and that could be implemented in C to make it fast and even special cased to further speed it up.

In a way, the current implementation focusses on runtime optimisation, not on a clean design.

[1st1]

I'll have a patch soon.

[1st1]

Please see the attached patch.

Couple of notes:

1. It adds a new type: 'coroutine' (PyCoro_Type). The implementation is still heavily based on generators, there are no changes there.

2. Unfortunately, a new opcode needs to be added - GET_YIELD_FROM_ITER. The idea is that YIELD_FROM from generators decorated with @coroutine must accept
   'async def' coroutines. But 'for' loops (iter(), etc) must throw an error on them. On the bright side, because of this new opcode, the ceval code became much easier to follow.

3. This change removes the need to use __instancecheck__ for 'async def' coroutines. BUT, it's still there for generators decorated with @coroutine. Stefan suggests to wrap generators in a thin wrapper with __await__ method, but I don't like this idea -- it will inevitably make existing asyncio code slower in 3.5; I'm -0.5 on this idea.

4. While this patch addresses initial request from Ben only partially (generator-based coroutines still require __instancecheck__), I believe that it's still a good idea to separate 'async def' coroutines & generators in 3.5. I'm fully confident that we shouldn't have any issues with that.

[scoder]

Looks good, simplifies the code visibly and makes the implementation quite similar to the one I wrote for Cython.

I like the fact that it separates generators from coroutines at an isinstance() level. The fact that "async def" functions currently have the same type as yield-based generators smells too much like an implementation detail to keep it visible at the language level.

[1st1]

Cleaned up the patch a little bit.

[1st1]

One more patch fixing minor bug in types.coroutine + a unittest for that. The patch should be ready for reviews.

[scoder]

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

[1st1]

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