Skip to content

Add docstrings to asyncio.TaskGroup #102560

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

Closed
JosephSBoyle opened this issue Mar 9, 2023 · 13 comments
Closed

Add docstrings to asyncio.TaskGroup #102560

JosephSBoyle opened this issue Mar 9, 2023 · 13 comments
Labels
3.11 only security fixes 3.12 only security fixes topic-asyncio type-feature A feature request or enhancement

Comments

@JosephSBoyle
Copy link
Contributor

JosephSBoyle commented Mar 9, 2023

Python 3.11 added asyncio.TaskGroup[0]. We should encourage users of the language to use this class by adding docstrings and type-hinting, particularly since it has wide applications for users wishing to refactor their existing async code.

I'm happy to work on this but any suggestions for what the docstrings should include is welcome.

[0] https://docs.python.org/3/library/asyncio-task.html

Linked PRs

@JosephSBoyle JosephSBoyle added the type-feature A feature request or enhancement label Mar 9, 2023
@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 9, 2023

Type hints for stdlib classes are generally added over at the typeshed repository (where contributions are very welcome!) rather than integrating them into the CPython source code directly. This is for a bunch of reasons -- some historical, but some that are still quite relevant. This doesn't mean that you won't find the occasional type hint in the CPython source code, but it isn't generally a goal to add type hints everywhere, nor to increase the level of "type annotation coverage" in the CPython repo.

Our annotations for asyncio.TaskGroup over at typeshed look pretty complete, so I think type checkers and IDEs should have everything they need for this class already in terms of type annotations :)

@github-project-automation github-project-automation bot moved this to Todo in asyncio Mar 9, 2023
@AlexWaygood AlexWaygood added 3.11 only security fixes 3.12 only security fixes labels Mar 9, 2023
@gvanrossum
Copy link
Member

That leaves docstrings. @JosephSBoyle do you want to contribute?

@AlexWaygood AlexWaygood changed the title Add docstrings and type hinting to asyncio.TaskGroup Add docstrings to asyncio.TaskGroup Mar 9, 2023
@JosephSBoyle
Copy link
Contributor Author

JosephSBoyle commented Mar 9, 2023

Ah, okay @AlexWaygood, thanks again for the explanation 🙇


Certainly, @gvanrossum. Here's a draft, largely adapted from the docs, with the main addition being the section about how once the context is exited the group's tasks are all either finished or cancelled.

class TaskGroup:
    """An asynchronous context manager for managing groups of tasks.

    Example use:

        async with asyncio.TaskGroup() as group:
            task1 = group.create_task(some_coroutine(...))
            task2 = group.create_task(other_coroutine(...))
        
        print("Both tasks have completed now.")

    All tasks are awaited when the context manager exits.

    Once the context has been exited it's guaranteed that no tasks
    from the group remain alive. That is, all tasks are guaranteed
    to have either finished successfully or have been cancelled.

    Any exceptions other than asyncio.CancelledError's raised within
    tasks cause each of the remaining tasks to be cancelled and the
    context to be exited.

    Once all tasks have finished, if any tasks have failed with an
    exception other than asyncio.CancelledError, those exceptions are
    combined in an ExceptionGroup or BaseExceptionGroup and re-raised.
    """
   
    ...
   
    def create_task(self, coro, *, name=None, context=None):
        """Schedule a coroutine and return a task representing it.
        
        :raises RuntimeError: if the task cannot be created.
        :returns: the newly created Task instance.
        """

@gvanrossum
Copy link
Member

Could you turn that into a PR? I think it would be okay if it was a little shorter. And we don't use :raises or :returns: in the stdlib, really.

@SimpleArt
Copy link

I don't believe mentioning that it can raise a RuntimeError is very helpful for most users. If it should be added, then it should be added to the official documentation instead, which is where I would expect more thorough documentation to live. In contrast, I would prefer to keep doc-strings more concise. This makes it easier for it to fit in a pop-up window when hovering in an IDE and works more as a quick reminder of how something works when you're already familiar with it.

I would also suggest mentioning that the method matches asyncio.create_task, which is already how it's documented.

Something like this for instance:

    def create_task(self, coro, *, name=None, context=None):
        """Create a task in this task group and return it.
        This method matches asyncio.create_task().
        """

@JosephSBoyle
Copy link
Contributor Author

Hi @SimpleArt, good points - I've changed the docstring accordingly.

@gvanrossum
Copy link
Member

gvanrossum commented Mar 12, 2023 via email

@AlexWaygood
Copy link
Member

No please make it into a PR.

On Sun, Mar 12, 2023 at 5:45 AM JosephSBoyle @.> wrote: Hi @SimpleArt https://github.com/SimpleArt, good points - I've changed the docstring accordingly. — Reply to this email directly, view it on GitHub <#102560 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWCWMQ3KYLXAC777VI34WTW3XAPDANCNFSM6AAAAAAVVJQWV4 . You are receiving this because you were mentioned.Message ID: @.>
-- --Guido van Rossum (python.org/~guido)

@gvanrossum, Joseph has already made a PR — #102565. It's linked to this issue :)

@AlexWaygood
Copy link
Member

@gvanrossum, are you interested in backporting #102565, or can this be closed now?

@gvanrossum
Copy link
Member

Oh, yes, let's backport to 3.11, where this was first introduced.

@gvanrossum gvanrossum added the needs backport to 3.11 only security fixes label Mar 15, 2023
@gvanrossum
Copy link
Member

Huh, where's the bot?

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Mar 15, 2023
@hugovk hugovk removed the needs backport to 3.11 only security fixes label Mar 15, 2023
@hugovk
Copy link
Member

hugovk commented Mar 15, 2023

The backport labels work on PRs, not issues.

I added it to #102565 and the bot backported it as #102715.

miss-islington added a commit that referenced this issue Mar 15, 2023
(cherry picked from commit e94edab)

Co-authored-by: JosephSBoyle <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Mar 15, 2023
@gvanrossum
Copy link
Member

Thanks! I suspect oncoming dementia.

Fidget-Spinner pushed a commit to Fidget-Spinner/cpython that referenced this issue Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 only security fixes topic-asyncio type-feature A feature request or enhancement
Projects
Status: Done
Development

No branches or pull requests

6 participants