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

Deprecate-remove passing coroutine objects to asyncio.wait() #78971

Closed
1st1 opened this issue Sep 24, 2018 · 24 comments
Closed

Deprecate-remove passing coroutine objects to asyncio.wait() #78971

1st1 opened this issue Sep 24, 2018 · 24 comments
Labels
topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@1st1
Copy link
Member

1st1 commented Sep 24, 2018

BPO 34790
Nosy @rhettinger, @asvetlov, @serhiy-storchaka, @1st1, @miss-islington, @tirkarthi, @jack1142, @aeros
PRs
  • bpo-34790: Document how passing coroutines to asyncio.wait() can be confusing. #9543
  • [3.7] bpo-34790: [docs] Passing coroutines to asyncio.wait() can be confusing. (GH-9543) #9577
  • bpo-34790: Fix asyncio.wait() 3.8 whatsnew entry #16975
  • bpo-34790: Implement deprecation of passing coroutines to asyncio.wait() #16977
  • bpo-34790: add version of removal of explicit passing of coros to asyncio.wait's documentation #20008
  • bpo-34790: Remove passing coroutine objects to asyncio.wait() #31964
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2022-03-17.20:52:30.179>
    created_at = <Date 2018-09-24.16:42:33.302>
    labels = ['type-bug', 'expert-asyncio']
    title = 'Deprecate-remove passing coroutine objects to asyncio.wait()'
    updated_at = <Date 2022-03-17.20:52:30.179>
    user = 'https://github.com/1st1'

    bugs.python.org fields:

    activity = <Date 2022-03-17.20:52:30.179>
    actor = 'asvetlov'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-03-17.20:52:30.179>
    closer = 'asvetlov'
    components = ['asyncio']
    creation = <Date 2018-09-24.16:42:33.302>
    creator = 'yselivanov'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34790
    keywords = ['patch']
    message_count = 24.0
    messages = ['326265', '326271', '326272', '326390', '326392', '326393', '326395', '326397', '355443', '355448', '355450', '355451', '355457', '355483', '355487', '355488', '355491', '355603', '355605', '355609', '355611', '359036', '368795', '415446']
    nosy_count = 8.0
    nosy_names = ['rhettinger', 'asvetlov', 'serhiy.storchaka', 'yselivanov', 'miss-islington', 'xtreak', 'jack1142', 'aeros']
    pr_nums = ['9543', '9577', '16975', '16977', '20008', '31964']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue34790'
    versions = []

    @1st1
    Copy link
    Member Author

    1st1 commented Sep 24, 2018

    asyncio.wait() accepts coroutines, wraps them into Tasks, and later returns those implicitly created Tasks in (done, pending) sets. This is very confusing to new asyncio users and it's almost impossible to figure out what is going on. See the first PR to the docs for more info.

    Andrew, I think we should deprecate passing coroutines to asyncio.wait() in 3.8, and disallow that in 4.0.

    @1st1 1st1 added topic-asyncio type-bug An unexpected behavior, bug, or error labels Sep 24, 2018
    @serhiy-storchaka
    Copy link
    Member

    4.0 is too far. Why not disallow them in 3.10?

    @1st1
    Copy link
    Member Author

    1st1 commented Sep 24, 2018

    4.0 is too far. Why not disallow them in 3.10?

    What's the current plan? I thought it's going to be 3.8, 3.9, 4.0. Is there a PEP detailing this?

    @1st1
    Copy link
    Member Author

    1st1 commented Sep 25, 2018

    New changeset 996859a by Yury Selivanov in branch 'master':
    bpo-34790: [docs] Passing coroutines to asyncio.wait() can be confusing. (GH-9543)
    996859a

    @1st1 1st1 closed this as completed Sep 25, 2018
    @1st1
    Copy link
    Member Author

    1st1 commented Sep 25, 2018

    Actually, since Andrew also agrees that we need to deprecate passing coroutines to wait(), I'll keep this issue open until we add an actual DeprecationWarning in 3.8.

    @1st1 1st1 reopened this Sep 25, 2018
    @miss-islington
    Copy link
    Contributor

    New changeset 3cc9557 by Miss Islington (bot) in branch '3.7':
    bpo-34790: [docs] Passing coroutines to asyncio.wait() can be confusing. (GH-9543)
    3cc9557

    @1st1
    Copy link
    Member Author

    1st1 commented Sep 25, 2018

    PendingDeprecationWarning

    @serhiy-storchaka
    Copy link
    Member

    AFAIK there is no a plan for 4.0. But seems many core developers (including me) think that changing the major number should reflect some major changes like removing GIL. And Guido preferred 3.10 after 3.9.

    @aeros
    Copy link
    Contributor

    aeros commented Oct 27, 2019

    Actually, since Andrew also agrees that we need to deprecate passing coroutines to wait(), I'll keep this issue open until we add an actual DeprecationWarning in 3.8.

    Since 3.8 has been released and the deprecation notice is in the 3.8 whatsnew document, should we implement the warning in Lib/asyncio/tasks.py? If so, I can open a PR.

    PendingDeprecationWarning

    Also, it's not clear to me if this should be a DeprecationWarning or PendingDeprecationWarning. The most recent message from Yury in the issue suggests a PendingDeprecationWarning, but the actual entry in the documentation (https://docs.python.org/3/library/asyncio-task.html?highlight=asyncio%20wait#asyncio.wait) seems like it might imply that it would be a DeprecationWarning:

    Deprecated since version 3.8: If any awaitable in aws is a coroutine, it is automatically scheduled as a Task. Passing coroutines objects to wait() directly is deprecated as it leads to confusing behavior.

    @asvetlov
    Copy link
    Contributor

    I think we can add DeprecationWarning for 3.9.

    Since it is a) just a warning b) already marked as deprecated in docs -- the harm is minimal.

    Honestly, we just missed the issue when were prepared for 3.8

    @aeros
    Copy link
    Contributor

    aeros commented Oct 27, 2019

    I think we can add DeprecationWarning for 3.9.

    If we add the deprecation warning just for 3.9, would the removal release also be pushed forward?

    Honestly, we just missed the issue when were prepared for 3.8

    Yeah that's definitely understandable, there were quite a number of major changes made in 3.8. It was the first time I saw a deprecation clearly documented in a final release that didn't have an associated deprecation warning.

    As for 3.8, I would like to update the whatsnew entry to add a link to this bpo issue if that's okay, as it does not at the moment. I recently included the entry in a PR that added entries for multiple significant changes for asyncio in the 3.8, but I forgot to include the bpo link (mostly because I found it in the documentation, rather than from the bpo issue).

    @aeros
    Copy link
    Contributor

    aeros commented Oct 27, 2019

    It was the first time I saw a deprecation clearly documented in a final release that didn't have an associated deprecation warning.

    I want to clarify that this may be a more common occurrence than I'm realizing, I'm not entirely certain. Also it's not intended at all as a criticism, it just surprised me since it was the first time I saw it happen.

    @asvetlov
    Copy link
    Contributor

    If we add the deprecation warning just for 3.9, would the removal release also be pushed forward?

    Yes, deprecating in 3.9 with removal in 3.11 is fine.

    Regarding 3.8 release notes update -- not sure if it is needed flr docs-only change.

    In the current situation, we have so-called *soft deprecation*: bare coroutines are deprecated in docs without any code change. This is perfectly fine, we give our users extra time to prepare for changes.

    @aeros
    Copy link
    Contributor

    aeros commented Oct 27, 2019

    Regarding 3.8 release notes update -- not sure if it is needed flr docs-only change.

    I'm not certain if the entry is necessary; my main concern is just that it's already present in the 3.8 release notes/whatsnew without anywhere to look for further information.

    In the current situation, we have so-called *soft deprecation*: bare coroutines are deprecated in docs without any code change. This is perfectly fine, we give our users extra time to prepare for changes.

    So is something considered to be a "soft deprecation" if there is no code change? If so, I may have made a mistake by specifying a release in the entry I recently added for it (https://docs.python.org/3/whatsnew/3.8.html#deprecated):

    "The explicit passing of coroutine objects to asyncio.wait() has been deprecated and will be removed in version 3.10."

    I would propose to adjust this to:

    "The explicit passing of coroutine objects to asyncio.wait() has been deprecated. (Contributed by Yury Selivanov in :issue:`34790`.)"

    Then for 3.9 whatsnew, we could specify the release:

    "The explicit passing of coroutine objects to asyncio.wait() has been deprecated and will be removed in 3.11."

    Thoughts?

    @1st1
    Copy link
    Member Author

    1st1 commented Oct 27, 2019

    I'd add and will be removed in 3.11. now.

    @asvetlov
    Copy link
    Contributor

    lgtm

    @aeros
    Copy link
    Contributor

    aeros commented Oct 27, 2019

    Sounds good, I'll work on opening a PR.

    @aeros
    Copy link
    Contributor

    aeros commented Oct 29, 2019

    #61179 Is a simple fix for the asyncio.wait() whatsnew entry for 3.8. I'll implement the deprecation warning and add a 3.9 whatsnew entry in a separate PR, since those changes won't be backported.

    @aeros
    Copy link
    Contributor

    aeros commented Oct 29, 2019

    #61181 Implements the deprecation warning, adds tests, and adds the 3.9 whatsnew entry. Once this PR is finalized, I think this issue can be closed.

    @rhettinger
    Copy link
    Contributor

    Is this whatsnew/3.8.rst entry correct and complete?

    '''
    The :func:`asyncio.coroutine` :term:`decorator` is deprecated and will be removed in version 3.10. Instead of ``@asyncio.coroutine``, use :keyword:`async def` instead. (Contributed by Andrew Svetlov in :issue:`36921`.)
    '''

    @aeros
    Copy link
    Contributor

    aeros commented Oct 29, 2019

    Is this whatsnew/3.8.rst entry correct and complete?

    The :func:`asyncio.coroutine` :term:`decorator` is deprecated and will be removed in version 3.10. Instead of ``@asyncio.coroutine``, use :keyword:`async def` instead. (Contributed by Andrew Svetlov in :issue:`36921`.)

    It looks to be fully consistent with the asyncio docs: https://docs.python.org/3/library/asyncio-task.html#generator-based-coroutines. From the blame of the documentation, it seems like Yury added the deprecation notice (GH-9314) to that section with the listed release for removal as 4.0, and then changed it to 3.10 (GH-9579). At a later date, Andrew implemented the actual deprecation (GH-13346).

    So as far as I can tell, yes.

    @asvetlov
    Copy link
    Contributor

    New changeset 89aa7f0 by Andrew Svetlov (Kyle Stanley) in branch 'master':
    bpo-34790: Implement deprecation of passing coroutines to asyncio.wait() (GH-16977)
    89aa7f0

    @1st1
    Copy link
    Member Author

    1st1 commented May 13, 2020

    New changeset de92769 by jack1142 in branch 'master':
    bpo-34790: add version of removal of explicit passing of coros to asyncio.wait's documentation (bpo-20008)
    de92769

    @asvetlov asvetlov changed the title Deprecate passing coroutine objects to asyncio.wait() Deprecate-remove passing coroutine objects to asyncio.wait() Mar 17, 2022
    @asvetlov
    Copy link
    Contributor

    New changeset 903f0a0 by Andrew Svetlov in branch 'main':
    bpo-34790: Remove passing coroutine objects to asyncio.wait() (GH-31964)
    903f0a0

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-asyncio type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants