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

wants to merge 4 commits into
base: pr15-pr16-combined


Copy link

@oremanj oremanj commented Apr 5, 2018

This improves performance and offers cleaner tracebacks, although native generators continue to have the failure mode of 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 Apr 5, 2018
Copy link

@codecov 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/ 100% <100%> (ø) ⬆️
async_generator/_tests/ 100% <100%> (ø) ⬆️
async_generator/_tests/ 100% <100%> (ø) ⬆️
async_generator/ 100% <100%> (ø) ⬆️
async_generator/ 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.

oremanj added 3 commits Apr 5, 2018

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.
Copy link
Member Author

@oremanj 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?

Copy link

@njsmith 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
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.