Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability for @async_generator to produce a native async generator #17

Open
wants to merge 4 commits into
base: pr15-pr16-combined
Choose a base branch
from

Conversation

oremanj
Copy link
Member

@oremanj oremanj commented Apr 5, 2018

This improves performance and offers cleaner tracebacks, although native generators continue to have the failure mode of https://bugs.python.org/issue32526 and to provide not-very-clear exception messages when they're misused.

Since the semantics of native async generators are slightly different, this change has @async_generator continue to produce an old-style pure-Python AsyncGenerator by default, with warnings if it looks like bpo-32526 would change the behavior of code that's using the async generator. Users can say @async_generator_legacy to continue to get the old behavior without the warnings, or @async_generator_native to use a native generator where available; my thought is that the next release of async_generator can make @async_generator_native be the default.

Also add an ag_await attribute to legacy async generators, to match the behavior of native ones.

@oremanj oremanj requested a review from njsmith April 5, 2018 21:20
@codecov
Copy link

codecov bot commented Apr 5, 2018

Codecov Report

Merging #17 into pr15-pr16-combined will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           pr15-pr16-combined    #17    +/-   ##
==================================================
  Coverage                 100%   100%            
==================================================
  Files                       7      7            
  Lines                     991   1108   +117     
  Branches                   78    100    +22     
==================================================
+ Hits                      991   1108   +117
Impacted Files Coverage Δ
async_generator/_tests/test_async_generator.py 100% <100%> (ø) ⬆️
async_generator/_tests/conftest.py 100% <100%> (ø) ⬆️
async_generator/_tests/test_util.py 100% <100%> (ø) ⬆️
async_generator/_impl.py 100% <100%> (ø) ⬆️
async_generator/__init__.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df24add...e520187. Read the comment docs.

…unction

For example, `@trio.enable_ki_protection` requires this if you put it below `@async_generator`. We can't support native-generator-ization in this case, but we can at least avoid crashing.
@oremanj
Copy link
Member Author

oremanj commented May 2, 2018

I originally made this maximally paranoid about the minor behavioral differences, but on reflection I'm not sure that level of paranoia is necessary -- I think it would reasonable to make @async_generator produce native generators on CPython 3.6+ without a deprecation period, and continue to permit @async_generator(allow_native=False) to use the old pure-Python version always. That reduces the API footprint (exposing @async_generator_legacy and @async_generator_native feels like a wart to me). Thoughts?

@njsmith
Copy link
Member

njsmith commented Aug 1, 2018

See #16 (comment) for high-level review comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants