Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(104)

#24017: Implemenation of the PEP 492 - Coroutines with async and await syntax

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 7 months ago by victor.stinner
Modified:
2 years, 6 months ago
Reviewers:
yselivanov, ncoghlan, guido, scoder
CC:
gvanrossum, Nick Coghlan, scoder, haypo, asvetlov, alex.gronholm, devnull_psf.upfronthosting.co.za, Yury Selivanov
Visibility:
Public.

Patch Set 1 #

Total comments: 109

Patch Set 2 #

Patch Set 3 #

Patch Set 4 #

Total comments: 26

Patch Set 5 #

Total comments: 8

Patch Set 6 #

Total comments: 3

Patch Set 7 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Include/opcode.h View 1 2 3 4 5 6 1 chunk +68 lines, -67 lines 0 comments Download
Lib/opcode.py View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download
Lib/test/test_coroutines.py View 1 2 3 4 5 6 1 chunk +117 lines, -1 line 0 comments Download
Python/ceval.c View 1 2 3 4 5 6 1 chunk +28 lines, -0 lines 0 comments Download
Python/compile.c View 1 2 3 4 5 6 5 chunks +14 lines, -33 lines 0 comments Download
Python/opcode_targets.h View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 16
haypo
First review. https://bugs.python.org/review/24017/diff/14685/Include/ceval.h File Include/ceval.h (right): https://bugs.python.org/review/24017/diff/14685/Include/ceval.h#newcode26 Include/ceval.h:26: PyAPI_FUNC(void) PyEval_SetAsyncWrapper(PyObject *wrapper); It should be renamed ...
2 years, 7 months ago #1
Yury Selivanov
Hi Victor. Thanks for the review. I'm only half gone through it. I've pushed all ...
2 years, 7 months ago #2
Yury Selivanov
Victor, I've fixed all stuff in *.c and *.h files. I'll try to fix asyncio ...
2 years, 7 months ago #3
Yury Selivanov
http://bugs.python.org/review/24017/diff/14685/Lib/asyncio/base_events.py File Lib/asyncio/base_events.py (right): http://bugs.python.org/review/24017/diff/14685/Lib/asyncio/base_events.py#newcode350 Lib/asyncio/base_events.py:350: self.set_debug(False) See my comment below http://bugs.python.org/review/24017/diff/14685/Lib/asyncio/base_events.py#newcode1186 Lib/asyncio/base_events.py:1186: sys.set_async_wrapper(lambda gen: ...
2 years, 6 months ago #4
Nick Coghlan
The general structure of this patch looks reasonable to me, so I'm mainly providing recommendations ...
2 years, 6 months ago #5
Yury Selivanov
Thanks a lot for the review, Nick! I'll also leave a comment in the issue. ...
2 years, 6 months ago #6
Yury Selivanov
http://bugs.python.org/review/24017/diff/14807/Lib/test/test_grammar.py File Lib/test/test_grammar.py (right): http://bugs.python.org/review/24017/diff/14807/Lib/test/test_grammar.py#newcode1033 Lib/test/test_grammar.py:1033: def test_async_await(self): On 2015/05/10 05:20:46, Nick Coghlan wrote: > ...
2 years, 6 months ago #7
Nick Coghlan
Getting pretty close now I think. Major remaining concerns: * the rationale in the PEP ...
2 years, 6 months ago #8
Yury Selivanov
> Getting pretty close now I think. Major remaining concerns: Thanks, Nick, for a thorough ...
2 years, 6 months ago #9
gvanrossum
On 2015/05/11 22:08:44, Yury.Selivanov wrote: > > Getting pretty close now I think. Major remaining ...
2 years, 6 months ago #10
Nick Coghlan
The latest version looks good to me, and thanks Guido for the explanation of the ...
2 years, 6 months ago #11
gvanrossum
On 2015/05/12 01:59:22, Nick Coghlan wrote: [] > That raises the question of what we ...
2 years, 6 months ago #12
Nick Coghlan
On 2015/05/12 02:07:30, gvanrossum wrote: > (Also, even lightly testing would reveal the error if ...
2 years, 6 months ago #13
scoder_users.sourceforge.net
https://bugs.python.org/review/24017/diff/14816/Include/object.h File Include/object.h (right): https://bugs.python.org/review/24017/diff/14816/Include/object.h#newcode178 Include/object.h:178: typedef PyObject *(*aiternextfunc) (PyObject *); Is there a reason ...
2 years, 6 months ago #14
scoder_users.sourceforge.net
https://bugs.python.org/review/24017/diff/14816/Lib/_collections_abc.py File Lib/_collections_abc.py (right): https://bugs.python.org/review/24017/diff/14816/Lib/_collections_abc.py#newcode144 Lib/_collections_abc.py:144: Awaitable.register(Coroutine) A Coroutine isn't Awaitable unless it implements __await__(). ...
2 years, 6 months ago #15
scoder_users.sourceforge.net
2 years, 6 months ago #16
https://bugs.python.org/review/24017/diff/14816/Lib/types.py
File Lib/types.py (right):

https://bugs.python.org/review/24017/diff/14816/Lib/types.py#newcode56
Lib/types.py:56: raise TypeError('coroutine() expects a generator function')
I propose to use this wrapping code as a fallback for types.coroutine() in the
case that a Generator (ABC) is passed instead of a generator (yield):

  class types_coroutine(object):
    def __init__(self, gen):
        self._gen = gen

    class as_coroutine(object):
        def __init__(self, gen):
            self._gen = gen
            self.send = gen.send
            self.throw = gen.throw
            self.close = gen.close

        def __await__(self):
            return self._gen

    def __call__(self, *args, **kwargs):
        return self.as_coroutine(self._gen(*args, **kwargs))

Note that the resulting Awaitable Coroutine type is not an Iterable. Flagged
generators are (although I think they shouldn't be).
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7