Repository navigation
make asyncio.iscoroutinefunction a deprecated alias of inspect.iscoroutinefunction and remove asyncio.coroutines._is_coroutine #94912
Copy link
Copy link
Closed as not planned
Closed as not planned
Copy link
Labels
Description
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.iscoroutinefunction a deprecated alias of inspect.iscoroutinefunction and remove asyncio.coroutines._is_coroutine.
This is now possible with the recent removal of @asyncio.coroutine and support for AsyncMock and other duck-type functions in inspect.iscoroutinefunction
the only caveats are - what should happen to users of the asyncio.coroutines._is_coroutine mark? eg:
Lines 686 to 696 in 1c0cf0a
| @types.coroutine | |
| 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 (yield from awaitable.__await__()) | |
| _wrap_awaitable._is_coroutine = _is_coroutine | |
and all of these:
Lib/test/test_asyncio/test_base_events.py: m_socket.getaddrinfo._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_reader._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_writer._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_reader._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_writer._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_reader._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_writer._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: m_socket.getaddrinfo._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: m_socket.getaddrinfo._is_coroutine = False
Lib/test/test_asyncio/test_base_events.py: self.loop._add_reader._is_coroutine = False
Lib/test/test_asyncio/test_selector_events.py: self.loop.add_reader._is_coroutine = False
Lib/test/test_asyncio/test_subprocess.py: protocol.connection_made._is_coroutine = False
Lib/test/test_asyncio/test_subprocess.py: protocol.process_exited._is_coroutine = False
Lib/unittest/mock.py: mock._is_coroutine = asyncio.coroutines._is_coroutine
Lib/unittest/mock.py: # iscoroutinefunction() checks _is_coroutine property to say if an
Lib/unittest/mock.py: self.__dict__['_is_coroutine'] = asyncio.coroutines._is_coroutine
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
- StatusShow more project fieldsDone
Activity
[-]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`[/+]graingert commentedon Jul 17, 2022
changing
_wrap_awaitableto be: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__"]callablesgraingert commentedon Jul 17, 2022
I'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?graingert commentedon Jul 17, 2022
aha I've just checked and the
protocol.process_exited._is_coroutine = Falselines were made redundant in python/asyncio#459gvanrossum commentedon Jul 17, 2022
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?
graingert commentedon Jul 17, 2022
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
kumaraditya303 commentedon Jul 18, 2022
Before making it a deprecated alias, it would be better to investigate:
graingert commentedon Jul 18, 2022
how about if I just add a deprecation notice and leave the implementation alone?
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 suregvanrossum commentedon Jul 18, 2022
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.
graingert commentedon Jul 18, 2022
The alternative is
asyncio.iscoroutinefunctionstarts returning False when it used to return True, which will break user code in a silent waygvanrossum commentedon Jul 18, 2022
I 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.
graingert commentedon Jul 19, 2022
ok I've pushed that change
Fixed documentation by removing obsolete reference.
carltongibson commentedon Oct 29, 2022
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
gvanrossum commentedon Jul 25, 2023
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.
Lancetnik commentedon Jul 25, 2023
Okay, 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
gvanrossum commentedon Jul 25, 2023
I think it's better to have two decorators. At least two of the Zen phrases would apply: EIBTI and TOOWTDI.
Lancetnik commentedon Jul 25, 2023
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 use
wrapswhen 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.gvanrossum commentedon Jul 25, 2023
Let's see what reports we get from actual users of 3.12 once it is released.
carltongibson commentedon Jul 25, 2023
This seems equivalent to the discussion on #100317.
If you're expecting to wrap an
async deffunction, you should return anasync deffunction from the decorator:(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.Lancetnik commentedon Jul 25, 2023
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 function
kumaraditya303 commentedon Aug 11, 2024
Superseded by #122858