-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
GH-100108: Add async generators best practices section #141885
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
base: main
Are you sure you want to change the base?
GH-100108: Add async generators best practices section #141885
Conversation
|
@kumaraditya303 Could you please take a look? |
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
| Asynchronous generators best practices | ||
| ====================================== | ||
|
|
||
| By :term:`asynchronous generator` in this section we will mean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems obvious, by generator we typically mean the generator object not the function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw this clarification in few places (e.g. https://docs.python.org/3/glossary.html#term-asynchronous-generator), so I thought it was worth mentioning. I'm OK to remove this if you think so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience, the term "generator" is used equally for a generator function and for the iterator returned by calling a generator function. If you really only use it for the latter, then this clarification makes sense.
| Manually close the generator | ||
| ---------------------------- | ||
|
|
||
| If an asynchronous generator happens to exit early by :keyword:`break`, the caller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels too heavy for start, I would start it as "It is recommended to manually close the generator... " then in second para describe the issue with breaking early.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC the scenario(s) you are trying to describe is where the code iterating over the generator doesn't iterate till the end. If you're iterating over it using a for-loop (there are other ways!) that could be a break in that for-loop, or the task containing that for-loop getting canceled, or indeed an exception being raised in the for-loop body. But I'm not sure it's helpful to try to list all the reasons why this can happen -- maybe the most useful one to mention is an exception being raised during that for-loop?
(The example code clarifies a lot!)
|
I would appreciate a review from @willingc and @gvanrossum on this. |
gvanrossum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's great to add these explicitly to the docs!
I hope you don't mind that I have tried to correct your English grammar somewhat (mostly adding or subtracting "the").
| ====================================== | ||
|
|
||
| By :term:`asynchronous generator` in this section we will mean | ||
| an :term:`asynchronous generator iterator` that returned by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| an :term:`asynchronous generator iterator` that returned by | |
| an :term:`asynchronous generator iterator` that is returned by an |
| Asynchronous generators best practices | ||
| ====================================== | ||
|
|
||
| By :term:`asynchronous generator` in this section we will mean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my experience, the term "generator" is used equally for a generator function and for the iterator returned by calling a generator function. If you really only use it for the latter, then this clarification makes sense.
|
|
||
|
|
||
| Asynchronous generators best practices | ||
| ====================================== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd insert some friendly words here introducing the reader to the purpose of this section. Right now it looks rather stern.
| Manually close the generator | ||
| ---------------------------- | ||
|
|
||
| If an asynchronous generator happens to exit early by :keyword:`break`, the caller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC the scenario(s) you are trying to describe is where the code iterating over the generator doesn't iterate till the end. If you're iterating over it using a for-loop (there are other ways!) that could be a break in that for-loop, or the task containing that for-loop getting canceled, or indeed an exception being raised in the for-loop body. But I'm not sure it's helpful to try to list all the reasons why this can happen -- maybe the most useful one to mention is an exception being raised during that for-loop?
(The example code clarifies a lot!)
| will run and possibly raise exceptions or access context variables in an | ||
| unexpected context--perhaps after the lifetime of tasks it depends, or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| will run and possibly raise exceptions or access context variables in an | |
| unexpected context--perhaps after the lifetime of tasks it depends, or | |
| will run in an unexpected context -- perhaps after the lifetime of tasks it depends on, or |
| Then it is recomended to create async generators only after the event loop | ||
| has already been created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I follow how it follows from the previous paragraph (which describes that finalization of an async generator-iterator may be delayed if its body hasn't finished yet) to the recommendation of creating them only after an event loop exists.
| Avoid iterating and closing the same generator concurrently | ||
| ----------------------------------------------------------- | ||
|
|
||
| The async generators allow to be reentered while another |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The async generators allow to be reentered while another | |
| Async generators may to be reentered while another |
| ----------------------------------------------------------- | ||
|
|
||
| The async generators allow to be reentered while another | ||
| :meth:`~agen.aclose`/:meth:`~agen.aclose`/:meth:`~agen.aclose` call is in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is :meth:`~agen.aclose` repeated three times here?
| The async generators allow to be reentered while another | ||
| :meth:`~agen.aclose`/:meth:`~agen.aclose`/:meth:`~agen.aclose` call is in | ||
| progress. This may lead to an inconsistent state of the async generator | ||
| and cause errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| and cause errors. | |
| and can cause errors. |
| Therefore, it is recommended to avoid using the async generators in parallel | ||
| tasks or in the multiple event loops. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Therefore, it is recommended to avoid using the async generators in parallel | |
| tasks or in the multiple event loops. | |
| Therefore, it is recommended to avoid using async generators in parallel | |
| tasks or from multiple event loops. |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I'd also love to get @willingc's view on how to make this section have the right tone so as not to scare beginners away. |
Added section about best practices for async generators.
asynciodocs #100108📚 Documentation preview 📚: https://cpython-previews--141885.org.readthedocs.build/