GeezerVison is showing github.com — make `asyncio.iscoroutinefunction` a deprecated alias of `inspect.iscoroutinefunction` and remove `asyncio.coroutines._is_coroutine` · Issue #94912 · python/cpython
make asyncio.iscoroutinefunction a deprecated alias of inspect.iscoroutinefunction and remove asyncio.coroutines._is_coroutine #94912Descriptioncurrently 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:
|
@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
Activitychanged 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, 2022graingert commented on Jul 17, 2022 ContributorAuthor More actions changing _wrap_awaitable to 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_awaitable check in asyncio.ensure_future(...) isn't needed anymore, and this would prevent wrapping objects that have .__dict__["__await__"] callables graingert commented on Jul 17, 2022 ContributorAuthor More actions I'm pretty sure all of the assignments in the test suite of protocol.process_exited._is_coroutine = False etc, are redundant or worse, invalidated by #94050 Edit: yes they are redundant see https://github.com/python/cpython/pull/94926/files#r922839081
the mock._is_coroutine = asyncio.coroutines._is_coroutine use in unittest.mock is also probably safe to remove? graingert commented on Jul 17, 2022 ContributorAuthor More actions aha I've just checked and the protocol.process_exited._is_coroutine = False lines were made redundant in python/asyncio#459 gvanrossum commented on 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 commented on Jul 17, 2022 ContributorAuthor More actions
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 kumaraditya303 commented on Jul 18, 2022 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?
graingert commented on Jul 18, 2022 ContributorAuthor More actions
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.iscoroutinefunction now uses inspect._signature_is_functionlike and so it should 'just work', I've let them know and will work on a cython test case to make sure
gvanrossum commented on Jul 18, 2022
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. graingert commented on Jul 18, 2022 ContributorAuthor More actions
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.iscoroutinefunction starts returning False when it used to return True, which will break user code in a silent way gvanrossum commented on 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 commented on Jul 19, 2022 ContributorAuthor More actions
It should only warn when it's going to return True but the inspect function would return False.
ok I've pushed that change carltongibson commented on Oct 29, 2022
the only caveats are - what should happen to users of the asyncio.coroutines._is_coroutine mark?
The _is_coroutine marker (for later use with asyncio.iscorountinefunction()) is leveraged by asgiref and 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 inspect fails).
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! gvanrossum commented on 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. Reacted by Carlton Gibson Lancetnik commented on 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 commented on 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 commented on Jul 25, 2023 But we have a lot of inspect.iscoroutinefunction based 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 wraps when 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 by inspect.iscoroutinefunction correctly. gvanrossum commented on Jul 25, 2023 Let's see what reports we get from actual users of 3.12 once it is released. Reacted by Pastukhov Nikita and Carlton Gibson carltongibson commented on Jul 25, 2023 This seems equivalent to the discussion on #100317.
If you're expecting to wrap an async def function, you should return an async def function 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 markcorountinefunction at all. In those cases it seems reasonable to have to explicitly use the marker. Lancetnik commented on 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 commented on Aug 11, 2024
MetadataMetadataStatusShow more project fields StatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|