Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-84753: Make inspect.iscoroutinefunction() work with AsyncMock#94050
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
sileht commented Jun 21, 2022 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
ghost commented Jun 21, 2022 • edited by ghost
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by ghost
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Jun 21, 2022
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
sileht commented Jun 21, 2022
@vstinner Can you take a look? |
graingert commented Jun 21, 2022
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.
sileht commented Jun 21, 2022
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. |
graingert commented Jun 21, 2022
looks like this issue is because fromunittestimportmockimportinspectm=mock.AsyncMock() withmock.patch("inspect.isfunction", new=lambdax: True): assertinspect.iscoroutinefunction(m)Originally posted by @graingert in #84753 (comment) |
graingert commented Jun 21, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Could CallableMixin mess about with |
sileht commented Jun 24, 2022
I tried but this doesn't seem to work, I suspect this fall in |
graingert commented Jun 24, 2022
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 |
sileht commented Jun 24, 2022
According to the doc, the only difference is |
sileht commented Jun 24, 2022 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
But, I agree that my solution doesn't work. We should keep the |
sileht commented Jun 28, 2022
I found another solution, I think it's cleaner. |
Uh oh!
There was an error while loading. Please reload this page.
fe4fc4e to 970f727CompareLib/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
miss-islington commented Jun 30, 2022
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 <lukasz@langa.pl> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
bedevere-bot commented Jun 30, 2022
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 <lukasz@langa.pl> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
bedevere-bot commented Jun 30, 2022
GH-94461 is a backport of this pull request to the 3.10 branch. |
sileht commented Jun 30, 2022
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 <lukasz@langa.pl> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
…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 <lukasz@langa.pl> (cherry picked from commit 4261b6b) Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
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.
tirkarthi commented Aug 23, 2022
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
asynciofix forAsyncMockin inspect module andmake
asyncio.iscoroutinefunctionan alias ofinspect.iscoroutinefunction.