-
-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
gh-84753: Make inspect.iscoroutinefunction() work with AsyncMock #94050
Conversation
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
@vstinner Can you take a look? |
this shouldn't be needed AsyncMock already sets Lines 2175 to 2178 in 5fcfdd8
|
Lib/inspect.py
Outdated
@@ -406,12 +406,15 @@ def isgeneratorfunction(obj): | |||
See help(isfunction) for a list of attributes.""" | |||
return _has_code_flag(obj, CO_GENERATOR) | |||
|
|||
# A marker for iscoroutinefunction. | |||
_is_coroutine = object() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this here instead of re-using asyncio.coroutines._is_coroutine
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it from asyncio to inspect as the code using it is now located in inspect module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_is_coroutine
feels like a property of asyncio, not inspect. I think it should stay where it is.
The test cases I added passed was passing for asyncio version, but was not for the inspect version. So something doesn't work as expected. |
looks like this issue is because from unittest import mock
import inspect
m = mock.AsyncMock()
with mock.patch("inspect.isfunction", new=lambda x: True):
assert inspect.iscoroutinefunction(m) Originally posted by @graingert in #84753 (comment) |
Could CallableMixin mess about with |
I tried but this doesn't seem to work, I suspect this fall in |
I'm relying on inspect.iscoroutinefunction to reveal things that really really are Coroutine Functions, partials or methods and asyncio.iscoroutinefunction to reveal things that are Duck typed as a Coroutine Function |
According to the doc, the only difference is |
But, I agree that my solution doesn't work. We should keep the |
I found another solution, I think it's cleaner. |
fe4fc4e
to
970f727
Compare
Lib/inspect.py
Outdated
return False | ||
return bool(f.__code__.co_flags & flag) | ||
if isfunction(f) or _signature_is_functionlike(f): | ||
return bool(f.__code__.co_flags & flag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try to preserve current coding style? Return early with False if a condition is false? Pseudo-code:
if not isfunction(f): return False
if not _signature_is_functionlike(f): return False
return bool(f.__code__.co_flags & flag)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
pythonGH-94050) The inspect version was not working with unittest.mock.AsyncMock. The fix introduces special-casing of AsyncMock in `inspect.iscoroutinefunction` equivalent to the one performed in `asyncio.iscoroutinefunction`. Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <[email protected]>
GH-94460 is a backport of this pull request to the 3.11 branch. |
pythonGH-94050) The inspect version was not working with unittest.mock.AsyncMock. The fix introduces special-casing of AsyncMock in `inspect.iscoroutinefunction` equivalent to the one performed in `asyncio.iscoroutinefunction`. Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <[email protected]>
GH-94461 is a backport of this pull request to the 3.10 branch. |
Thanks all! That was fast 🚀 |
…94050) (GH-94461) The inspect version was not working with unittest.mock.AsyncMock. The fix introduces special-casing of AsyncMock in `inspect.iscoroutinefunction` equivalent to the one performed in `asyncio.iscoroutinefunction`. Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <[email protected]>
…94050) (GH-94460) The inspect version was not working with unittest.mock.AsyncMock. The fix introduces special-casing of AsyncMock in `inspect.iscoroutinefunction` equivalent to the one performed in `asyncio.iscoroutinefunction`. Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <[email protected]>
The wording used in pythonGH-94050 suggests narrower scope than what was actually implemented. This commit clarifies the full impact of pythonGH-94050 in the change log.
This change seems to have caused a regression reported in #96127 |
The inspect version was not working with
unittest.mock.AsyncMock
.This change moves the
asyncio
fix forAsyncMock
in inspect module andmake
asyncio.iscoroutinefunction
an alias ofinspect.iscoroutinefunction
.