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

Use iscoroutinefunction from inspect not asyncio #359

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AdamWill
Copy link
Contributor

@AdamWill AdamWill commented Nov 5, 2024

Python 3.14 will deprecate asyncio.iscoroutinefunction: python/cpython#122875

inspect.iscoroutinefunction exists since 3.5 and our baseline is 3.8, so we can just use it unconditionally.

Using a wrapper with @asyncio.coroutine in get wasn't necessary (the future from asyncio.ensure_future is awaitable, and the wrapper doesn't do anything asynchronous), so the logic can be simplified to just call asyncio.ensure_future (to schedule the task and store the result when it's available).

Python 3.14 will deprecate asyncio.iscoroutinefunction:
python/cpython#122875

inspect.iscoroutinefunction exists since 3.5 and our baseline
is 3.8, so we can just use it unconditionally.

Using a wrapper with @asyncio.coroutine in __get__ wasn't
necessary (the future from asyncio.ensure_future is awaitable,
and the wrapper doesn't do anything asynchronous), so the
logic can be simplified to just call asyncio.ensure_future
(to schedule the task and store the result when it's
available).
@AdamWill
Copy link
Contributor Author

AdamWill commented Nov 5, 2024

This is a rebase of #267 , but it turns out to be much simpler against 2.0.1. Skipping test_coroutine_cached_property.py as the original did no longer appears to be necessary - arguably that test could now be removed as we're targeting a 3.8 baseline, but it seems like that could be a separate PR. @encukou for info, I've kept you as the author of this but can change it to me if you don't want your name on it any more :)

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.

2 participants