Repository navigation
make asyncio.iscoroutinefunction a deprecated alias of inspect.iscoroutinefunction and remove asyncio.coroutines._is_coroutine #94912
Description
Activity
- changed the title
[-]make asyncio.iscoroutinefunction a deprecated alias of inspect.iscoroutinefunction and remove asyncio.coroutines._is_coroutine[/-][+]make `asyncio.iscoroutinefunction` a deprecated alias of `inspect.iscoroutinefunction` and remove `asyncio.coroutines._is_coroutine`[/+]on Jul 17, 2022 changing
_wrap_awaitableto be:async def _wrap_awaitable(awaitable): """Helper for asyncio.ensure_future(). Wraps awaitable (an object with __await__) into a coroutine that will later be wrapped in a Task by ensure_future(). """ return await awaitable
would have a few subtle changes - eg the
if not called_wrap_awaitablecheck inasyncio.ensure_future(...)isn't needed anymore, and this would prevent wrapping objects that have.__dict__["__await__"]callablesI'm pretty sure all of the assignments in the test suite ofEdit: yes they are redundant see https://github.com/python/cpython/pull/94926/files#r922839081protocol.process_exited._is_coroutine = Falseetc, are redundant or worse, invalidated by #94050the
mock._is_coroutine = asyncio.coroutines._is_coroutineuse inunittest.mockis also probably safe to remove?aha I've just checked and the
protocol.process_exited._is_coroutine = Falselines were made redundant in python/asyncio#459So that's the ancient external asyncio project, which nobody should use any more. Does the corresponding code (or corresponding commits) exist in the stdlib version?
So that's the ancient external asyncio project, which nobody should use any more. Does the corresponding code (or corresponding commits) exist in the stdlib version?
Yep, the fix was ported everywhere but the now redundant workarounds were left in, there's more info here https://github.com/python/cpython/pull/94926/files#r922839081
Before making it a deprecated alias, it would be better to investigate:
- How much code will be affected by this change?
- How this interacts with cython coroutines?
Before making it a deprecated alias, it would be better to investigate:
* How much code will be affected by this change?how about if I just add a deprecation notice and leave the implementation alone?
* How this interacts with cython coroutines?inspect.iscoroutinefunctionnow usesinspect._signature_is_functionlikeand so it should 'just work', I've let them know and will work on a cython test case to make surehow about if I just add a deprecation notice and leave the implementation alone?
That's rarely a good strategy. Many people don't read the docs and keep using the function (if they are using it) and so once the deprecated function is deleted their code breaks without warning.
how about if I just add a deprecation notice and leave the implementation alone?
That's rarely a good strategy. Many people don't read the docs and keep using the function (if they are using it) and so once the deprecated function is deleted their code breaks without warning.
The alternative is
asyncio.iscoroutinefunctionstarts returning False when it used to return True, which will break user code in a silent wayI thought we were talking about putting a warning in it? It should only warn when it's going to return True but the inspect function would return False.
It should only warn when it's going to return True but the inspect function would return False.
ok I've pushed that change
- added a commit that references this issue
on Oct 28, 2022 the only caveats are - what should happen to users of the asyncio.coroutines._is_coroutine mark?
The
_is_coroutinemarker (for later use withasyncio.iscorountinefunction()) is leveraged byasgirefand Django to mark sync functions that return an awaitable, and are otherwise not detectable as such.Is there an official way to do this? (I need to dig into the inspect version of the check to see if we can mimic what we're doing already, but on the initial pass substituting in
inspectfails).If not can we pause before removing it?
@gvanrossum had #67707 (comment) — which suggests just calling the thing and seeing if it returned a coroutine object, but in Django's case we have a middleware and view functions which we're adapting for later use. Having a way of saying, No this is a coroutine function just by inspecting is pretty essential. 😬
Thanks!
19 remaining items
We are way too late for feature freeze, and I don't feel this is important enough to warrant changing course. Anyway, I'd rather have the code that adds the flag and the code that checks the flag in the same place, rather than having them spread across two quite unrelated modules.
Reacted by Carlton GibsonOkay, this is just a thought, not a problem. But what are you thinking about adding similar functionality to the
wraps?Maybe via a parameter (but the default behavior is better I think):
@wraps(func, mark_async=True) ....This decorator is just a sugar for decorators writing, so it can be very helpfull to add this feature here too
I think it's better to have two decorators. At least two of the Zen phrases would apply: EIBTI and TOOWTDI.
But we have a lot of
inspect.iscoroutinefunctionbased tools: all of these tools use the same decorator to wrap both async and sync functions.If we were to decorate the function before using them, we might break this check in a not-so-obvious way.
However, almost all users still usewrapswhen writing their own decorators. If this decorator automatically labels wrappers with the appropriate type, library developers will be guaranteed that the function that comes into their decorator is defined byinspect.iscoroutinefunctioncorrectly.Let's see what reports we get from actual users of 3.12 once it is released.
Reacted by Pastukhov Nikita and Carlton GibsonThis seems equivalent to the discussion on #100317.
If you're expecting to wrap an
async deffunction, you should return anasync deffunction from the decorator:import inspect from functools import wraps def decorator(func): @wraps(func) async def wrapper(*args, **kwargs): return func(*args, **kwargs) return wrapper @decorator async def some_func(): pass assert inspect.iscoroutinefunction(some_func)(That already works)
It's only in those (presumably rare) case where you need a sync function to return an async function (and in Django's case that's only where we need to handle both types on the same code path, and it's not recommended™ if you can avoid it) that you need
markcorountinefunctionat all. In those cases it seems reasonable to have to explicitly use the marker.Yep, I am as a OSS developer talking about cases when u need to wrap any function by sync function. It's not the Django only case: FastAPI, tenacity, loguru, etc uses the one decorator to wrap any function
So it could be a better decision to implement this functionality in the regular 'wraps', not in the special functionSuperseded by #122858
currently asyncio.iscoroutinefunction and inspect.iscoroutinefunction behave differently in confusing and hard to document ways. It's possible to bring them into alignment but I think it would be better to make
asyncio.iscoroutinefunctiona deprecated alias ofinspect.iscoroutinefunctionand removeasyncio.coroutines._is_coroutine.This is now possible with the recent removal of
@asyncio.coroutineand support for AsyncMock and other duck-type functions in inspect.iscoroutinefunctionthe only caveats are - what should happen to users of the
asyncio.coroutines._is_coroutinemark? eg:cpython/Lib/asyncio/tasks.py
Lines 686 to 696 in 1c0cf0a
and all of these: