FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-36921: Deprecate @coroutine for sake of async def by asvetlov · Pull Request #13346 · python/cpython · GitHub

/ cpython Public

bpo-36921: Deprecate @coroutine for sake of async def - #13346

Merged
miss-islington merged 15 commits into
python:masterfrom
asvetlov:deprecate-coroutine
May 16, 2019
Merged

bpo-36921: Deprecate @coroutine for sake of async def#13346
miss-islington merged 15 commits into
python:masterfrom
asvetlov:deprecate-coroutine

Conversation

asvetlov commented May 15, 2019
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

The second attempt. Now deprecate @coroutine only, keep yield from fut as is.

https://bugs.python.org/issue36921

Comment thread Lib/test/test_typing.py
@asyncio.coroutine
def __anext__(self) -> T_a:
data = yield from self.value
async def __anext__(self) -> T_a:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The same functionality but without @coroutine

ilevkivskyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The typing part LGTM.

Comment thread Lib/asyncio/coroutines.py Outdated
gvanrossum removed their request for review May 15, 2019 19:45
Co-Authored-By: Matthias Bussonnier <bussonniermatthias@gmail.com>

Carreau commented May 15, 2019

Copy link
Copy Markdown
Contributor

want to update asyncio-task.rst to use the .. deprecated-removed:: 3.8 3.10 directive ?

Carreau commented May 15, 2019

Copy link
Copy Markdown
Contributor

Also side-question (and maybe this is better suited for bpo , should asyncio.iscoroutine and asyncio.iscoroutinefunction be deprecated at the same time ?

Because presumably once @coroutine is removed, the two other have quasy no chance of encountering a coroutine that would not be properly detected by inspect.iscoroutine.

I'm thinking they are still useful from inter-library compatibility and across python version; so maybe deprecate as well but remove even later ?

Comment thread Lib/asyncio/coroutines.py Outdated
Comment thread Lib/asyncio/locks.py Outdated
Comment thread Lib/asyncio/locks.py Outdated

1st1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Overall LGTM. Please fix the few nits I commented on.

1st1 commented May 15, 2019

Copy link
Copy Markdown
Member

Also side-question (and maybe this is better suited for bpo , should asyncio.iscoroutine and asyncio.iscoroutinefunction be deprecated at the same time ?

We can deprecate them when we remove @asyncio.coroutine (in 3.9?)

Co-Authored-By: Yury Selivanov <yury@magic.io>
asvetlov and others added 2 commits May 16, 2019 09:20
Co-Authored-By: Yury Selivanov <yury@magic.io>

Copy link
Copy Markdown
Contributor Author

@caavery the funny thing that @coroutine is already deprecated in docs. Now we have the support of this statement explicitly in code.
Removal is scheduled for 3.10, that's fine.
I agree with @1st1 that asyncio.iscoroutine and asyncio.iscoroutinefunction should be deprecated later.
As the maintainer of several asyncio based libraries, I want a smooth transition path.
My libraries don't use @coroutine already but sometimes accept async callbacks and calls asyncio checkers for them. I want to support user-written old-style coros in my libraries as long as possible (but discourage them).
Basically the same as asyncio itself does.
Deprecation checkers after removal the root functionality absolutely makes sense to me. We can consider deprecation in 3.9 if you want but I pretty sure we shouldn't do it in 3.8.

This decorator should not be used for :keyword:`async def`
coroutines.

.. deprecated-removed:: 3.8 3.10

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Replace free-form deprecation text with more standardized one.

asvetlov commented May 16, 2019
edited
Loading

Copy link
Copy Markdown
Contributor Author

The last call for a review.
I have a plan to merge the PR in a day.

1st1 commented May 16, 2019

Copy link
Copy Markdown
Member

The last call for a review.

I think it's ready.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL