Andrew Svetlov added the comment: On Sun, May 10, 2015 at 7:21 PM, Yury Selivanov <rep...@bugs.python.org> wrote: > > Yury Selivanov added the comment: > >> Review sent - very nice work on this Yury. > > Thanks a lot, Nick! > > Highlights: > >> * I concur with Stefan that we should have a full PyCoroutineMethods struct >> at the C level, with a "tp_as_coroutine" pointer to that replacing the >> current tp_reserved slot > > Do you think that tp_as_async is a better name? (I explained my point of > view in code review comments) > > Also, do we need slots for __aenter__ and __aexit__? We don't have slots for > regular context manager protocol, fwiw. > >> * I also concur with Stefan about adding a Coroutine ABC > > I will. We definitely need it. > >> * PyType_FromSpec (and typeslots.h) will need updating once we agree on a >> slot structure (with my recommendation being "define C level slots for all >> of the new PEP 492 methods") > >> * I found CO_COROUTINE/CO_NATIVE_COROUTINE confusing as a reader of the >> implementation, as they only told me how the objects were defined, rather >> than telling me why I should care. Based on what I gleaned of their intended >> purpose from reading the implementation, I suggest switching this to instead >> use CO_COROUTINE (set for all coroutines, regardless of how they were >> defined) and CO_ITERABLE_COROUTINE (set only for those coroutines that also >> support iteration), and adjusting the naming of other APIs accordingly. > > I agree that CO_COROUTINE is something that we should use for 'async def' > functions (instead of CO_NATIVE_COROUTINE). However, CO_ITERABLE_COROUTINE > sounds a bit odd to me, as generator-based coroutines (at least in asyncio) > aren't supposed to be iterated over. How about CO_GENBASED_COROUTINE flag? >
Maybe CO_ASYNC_COROUTINE and CO_OLDSTYLE_COROUTINE? This is wild proposal, feel free to ignore it. > >> * I found the names of the WITH_CLEANUP_ENTER and WITH_CLEANUP_EXIT >> bytecodes misleading, as they don't refer to the corresponding context >> management phases - they're both related to the "exit" phase. >> WITH_CLEANUP_START and WITH_CLEANUP_FINISH should be clearer for readers >> (both of the implementation and of the disassembled bytecode). > > Big +1. Your names are much better. > > ---------- > > _______________________________________ > Python tracker <rep...@bugs.python.org> > <http://bugs.python.org/issue24017> > _______________________________________ ---------- _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue24017> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
