gh-105331: Fix asyncio.sleep() bug#105641
Uh oh!
There was an error while loading. Please reload this page.
Conversation
weijay0804 requested review from 1st1, asvetlov, gvanrossum, kumaraditya303 and willingc as code owners
bedevere-bot commented Jun 11, 2023
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
Lib/asyncio/tasks.py Outdated
| """Create a function suitable for use as a task factory on an event-loop. | ||
| Example usage: | ||
| Example usage: |
There was a problem hiding this comment.
Please fix the indentation change you (accidentally?) applied to much of this docstring. (Maybe your editor inserted tabs instead of spaces?)
Lib/asyncio/tasks.py Outdated
| """Coroutine that completes after a given time (in seconds).""" | ||
| # check delay value is not nan | ||
| importmath |
There was a problem hiding this comment.
Just move this import to the toplevel, sorted between the rest of the imports.
Lib/asyncio/tasks.py Outdated
| # check delay value is not nan | ||
| importmath | ||
| ifmath.isnan(delay): | ||
| raiseValueError("Invalid value NaN (not a number)") |
There was a problem hiding this comment.
Suggestion:
| raiseValueError("Invalid value NaN (not a number)") | |
| raiseValueError("Invalid delay: NaN (not a number)") |
| @@ -0,0 +1 @@ | |||
| fix asyncio.sleep(float('nan')) does not raise ValueErro problem | |||
There was a problem hiding this comment.
Please try harder here. The text requires proper punctuation, capitalization, markup, and it should describe the change in behavior rather than just stating "fix the problem".
bedevere-bot commented Jun 11, 2023
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
weijay0804force-pushed the fix-issue-105331 branch from b3eed28 to 408814dCompareweijay0804force-pushed the fix-issue-105331 branch from c0fa428 to 9bd7cedCompare
Lib/asyncio/tasks.py Outdated
| """Coroutine that completes after a given time (in seconds).""" | ||
| # check delay value is not nan | ||
| ifmath.isnan(delay): |
There was a problem hiding this comment.
Move this check after delay <= 0 check to not slow down the common case of sleep(0).
Misc/NEWS.d/next/Core and Builtins/2023-06-11-09-14-30.gh-issue-105331.nlZvoW.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
…e-105331.nlZvoW.rst I think this way is more clearer. Thank you! Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
gvanrossum left a comment
gvanrossum left a comment There was a problem hiding this comment.
@kumaraditya303 Do you agree?
kumaraditya303 commented Jun 12, 2023
I pushed some minor tweaks, now LGTM. |
* main: pythongh-105540: Fix code generator tests (python#105707) pythongh-105375: Explicitly initialise all{Pickler,Unpickler}Object fields (python#105686) pythongh-105331: Change `asyncio.sleep` to raise ``ValueError` for nan (python#105641) Remove support for legacy bytecode instructions (python#105705)
4 participants
Issue: gh-105331:
fix asyncio.sleep(float('nan')) does not raise ValueError