Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
typing tests: remove some unnecessary uses ofexec()#119005
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Lib/test/test_typing.py Outdated
| exec('async def g(): yield 0', globals(), ns) | ||
| g = ns['g'] | ||
| async def g(): yield 0 | ||
| g = g() |
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.
In the old code here g was the function, now it's the coroutine returned from the function.
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.
good catch. Sorry, that was sloppy.
AlexWaygoodMay 13, 2024 • 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.
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.
Although on second thought... doesn't it make more sense to test the coroutine returned from the function here? I think the test might be buggy as-written.
It passes regardless, and this PR wasn't meant to make any semantic changes to the tests, so I'm happy to leave it as-is for now (I pushed6f25644), but it looks odd to me
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.
Yeah, it's odd that the passes both with and without theg(). Might be worth looking into whether we can make the test more effective.
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.
It makes sense that it passes both with and without theg(), becauseg is only used to check that the [coroutine-function or coroutine] isnot an instance of the customAsyncGenerator subclass here:
cpython/Lib/test/test_typing.py
Lines 7459 to 7460 in7d7eec5
| self.assertNotIsInstance(type(g),G) | |
| self.assertNotIsInstance(g,G) |
And of course, it is indeed true that neither a coroutine nor a coroutine function will be an instance of the customAsyncGenerator subclass
Thanks@AlexWaygood for the PR, and@JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
(cherry picked from commita9328e2)Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
(cherry picked from commita9328e2)Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
GH-119038 is a backport of this pull request to the3.13 branch. |
GH-119039 is a backport of this pull request to the3.12 branch. |
This is a forward-port ofpython/typing_extensions#219