Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ambv merged 2 commits intopython:mainfromsileht:iscoroutinefix
Jun 30, 2022

Conversation

@sileht
Copy link
Contributor

@silehtsileht commentedJun 21, 2022•
edited by bedevere-bot
Loading

The inspect version was not working withunittest.mock.AsyncMock.

This change moves theasyncio fix forAsyncMock in inspect module and
makeasyncio.iscoroutinefunction an alias ofinspect.iscoroutinefunction.

@ghost
Copy link

ghost commentedJun 21, 2022•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@sileht
Copy link
ContributorAuthor

@vstinner Can you take a look?

@graingert
Copy link
Contributor

this shouldn't be needed AsyncMock already sets

code_mock=NonCallableMock(spec_set=CodeType)
code_mock.co_flags=inspect.CO_COROUTINE
self.__dict__['__code__']=code_mock

Copy link
Contributor

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-usingasyncio.coroutines._is_coroutine?

Copy link
ContributorAuthor

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.

Copy link
Contributor

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
Copy link
ContributorAuthor

this shouldn't be needed AsyncMock already sets

code_mock=NonCallableMock(spec_set=CodeType)
code_mock.co_flags=inspect.CO_COROUTINE
self.__dict__['__code__']=code_mock

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
Copy link
Contributor

looks like this issue is becausemock.AsyncMock() doesn't passinspect.isfunction:

fromunittestimportmockimportinspectm=mock.AsyncMock()withmock.patch("inspect.isfunction",new=lambdax:True):assertinspect.iscoroutinefunction(m)

Originally posted by@graingert in#84753 (comment)

@graingert
Copy link
Contributor

graingert commentedJun 21, 2022•
edited
Loading

Could CallableMixin mess about with__instancecheck__/__subclasscheck__/__class__ to passisinstance(AsyncMock(), types.FunctionType)?

@sileht
Copy link
ContributorAuthor

Could CallableMixin mess about with__instancecheck__/__subclasscheck__/__class__ to passisinstance(AsyncMock(), types.FunctionType)?

I tried but this doesn't seem to work, I suspect this fall inisinstance C optimisation and__instancecheck__ and friends are never called.

@graingert
Copy link
Contributor

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

AlexWaygood reacted with thumbs up emoji

@sileht
Copy link
ContributorAuthor

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 isasyncio.iscoroutinefunction also works for deprecated generator based coroutine
This legacy coroutine will(have ?) be removed for 3.11, so now both functions should be the same.

@sileht
Copy link
ContributorAuthor

sileht commentedJun 24, 2022•
edited
Loading

But, I agree that my solution doesn't work. We should keep the_is_coroutine hack as-is until generated coroutine are removed and found another solution to makeisfunction pass forAsyncMock().

@sileht
Copy link
ContributorAuthor

I found another solution, I think it's cleaner.

@silehtsilehtforce-pushed theiscoroutinefix branch 3 times, most recently fromfe4fc4e to970f727CompareJune 28, 2022 17:56
Copy link
Member

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 Falseif not _signature_is_functionlike(f): return Falsereturn bool(f.__code__.co_flags & flag)

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

done

@ambvambv changed the titlegh-84753: make inspect.iscoroutinefunction() works with AsyncMockgh-84753: Make inspect.iscoroutinefunction() work with AsyncMockJun 30, 2022
@ambvambv added needs backport to 3.10only security fixes needs backport to 3.11only security fixes labelsJun 30, 2022
@ambvambv merged commit4261b6b intopython:mainJun 30, 2022
@miss-islington
Copy link
Contributor

Thanks@sileht for the PR, and@ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJun 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 oneperformed in `asyncio.iscoroutinefunction`.Co-authored-by: Łukasz Langa <lukasz@langa.pl>(cherry picked from commit4261b6b)Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
@bedevere-bot
Copy link

GH-94460 is a backport of this pull request to the3.11 branch.

@bedevere-botbedevere-bot removed the needs backport to 3.11only security fixes labelJun 30, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestJun 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 oneperformed in `asyncio.iscoroutinefunction`.Co-authored-by: Łukasz Langa <lukasz@langa.pl>(cherry picked from commit4261b6b)Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
@bedevere-botbedevere-bot removed the needs backport to 3.10only security fixes labelJun 30, 2022
@bedevere-bot
Copy link

GH-94461 is a backport of this pull request to the3.10 branch.

@sileht
Copy link
ContributorAuthor

Thanks all! That was fast 🚀

ambv pushed a commit that referenced this pull requestJun 30, 2022
…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 oneperformed in `asyncio.iscoroutinefunction`.Co-authored-by: Łukasz Langa <lukasz@langa.pl>(cherry picked from commit4261b6b)Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
ambv pushed a commit that referenced this pull requestJun 30, 2022
…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 oneperformed in `asyncio.iscoroutinefunction`.Co-authored-by: Łukasz Langa <lukasz@langa.pl>(cherry picked from commit4261b6b)Co-authored-by: Mehdi ABAAKOUK <sileht@sileht.net>
ambv added a commit to ambv/cpython that referenced this pull requestJul 5, 2022
The wording used inpythonGH-94050 suggests narrower scope than what was actuallyimplemented. This commit clarifies the full impact ofpythonGH-94050 in the changelog.
@tirkarthi
Copy link
Member

This change seems to have caused a regression reported in#96127

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@vstinnervstinnervstinner left review comments

@graingertgraingertgraingert left review comments

@cjw296cjw296cjw296 approved these changes

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

+1 more reviewer

@jdjdjd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@sileht@bedevere-bot@graingert@miss-islington@tirkarthi@jd@vstinner@cjw296@ambv
[8]ページ先頭
©2009-2025 Movatter.jp