gh-98641: Don't portray asyncio.TaskGroup as a replacement for asyncio.gather in docs #98659
Conversation
The purpose of the comments is to rule out the implication that asyncio.TaskGroup is a drop-in replacement / better alternative to asyncio.gather(). Both have their idiomatic uses cases, and asyncio.gather() works well for tasks that do not schedule tasks themselves, especially when the caller needs to consume their result as soon as possible.
why-not-try-calmer
requested review from
1st1,
asvetlov and
gvanrossum
as code owners
|
Backports will be handled automatically after the PR against |
|
Oh, sorry; this is the PR against main. Your |
zware
changed the title
I apologize for the inconvenience. 🙏🏻 Between my fork, the Issue and the PR I probably misread something. |
Doc/library/asyncio-task.rst
Outdated
| wait for their completion is :class:`asyncio.TaskGroup`. While *TaskGroup* | ||
| provides strong safety guarantees for scheduling a nesting of subtasks, *gather* comes in handy | ||
| for tasks that do not schedule subtasks and need to have their results consumed eagerly | ||
| (i.e. when destructuring the result(s) into a tuple). |
There was a problem hiding this comment.
| wait for their completion is :class:`asyncio.TaskGroup`. While *TaskGroup* | |
| provides strong safety guarantees for scheduling a nesting of subtasks, *gather* comes in handy | |
| for tasks that do not schedule subtasks and need to have their results consumed eagerly | |
| (i.e. when destructuring the result(s) into a tuple). | |
| wait for their completion is :class:`asyncio.TaskGroup`. While *TaskGroup* | |
| provides strong safety guarantees for scheduling a nesting of subtasks, *gather* is useful | |
| for tasks that do not schedule subtasks and need to have their results consumed eagerly | |
| (i.e. when destructuring the result(s) into a tuple). |
Honestly I'm not sure I agree. If you need the results, you can just await the tasks. The big difference is that gather will not cancel remaining tasks once one task raises. Your description here doesn't call out that difference.
There was a problem hiding this comment.
I agree, my formulation didn't address this important point. Instead of committing your suggestion, I have tried to accomodate it, rewording this section of the documentation to better emphasize the difference and uses cases between TaskGroup and gather. I don't want to stretch the text too much as this is not meant to be a tutorial, but I am all ears if this can be improved even more.
|
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 And if you don't make the requested changes, you will be poked with soft cushions! |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @gvanrossum: please review the changes made to this pull request. |
gvanrossum
changed the title
|
Sorry, any news about this? |
| @@ -447,8 +448,15 @@ Running Tasks Concurrently | |||
| Tasks/Futures to be cancelled. | |||
|
|
|||
| .. note:: | |||
| A more modern way to create and run tasks concurrently and | |||
| wait for their completion is :class:`asyncio.TaskGroup`. | |||
| A new alternative to create and run tasks concurrently and | |||
There was a problem hiding this comment.
I don't think there should be this much information in a note.
There was a problem hiding this comment.
Thanks for your comment. It's difficult to turn into something actionable. Can you clarify what you'd remove, bearing in mind the purpose and arguments put forth in this PR?
There was a problem hiding this comment.
Honestly this belongs more to a FAQ entry than reference docs. But this note was already there, I suggest to only explain the case why gather should be used and keep it short something like "Use asyncio.gather if you want to concurrently run coroutines and cancellation of one does not affects others otherwise it is recommend to use TaskGroups". Feel free to word it better!
|
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 am closing this, we're not even in agreement on whether there is anything that should be changed here, let alone on how it should be changed. |
The purpose of the comments is to rule out the implication that asyncio.TaskGroup is a drop-in replacement / better alternative to asyncio.gather(). Both have their idiomatic uses cases, and asyncio.gather() works well for tasks that do not schedule tasks themselves, especially when the caller needs to consume their result as soon as possible.