Repository navigation
gh-105331: Fix asyncio.sleep() bug - #105641
kumaraditya303 merged 14 commits into
Conversation
Sorry, something went wrong.
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
Sorry, something went wrong.
| """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?)
Sorry, something went wrong.
| """Coroutine that completes after a given time (in seconds).""" | ||
|
|
||
| # check delay value is not nan | ||
| import math |
There was a problem hiding this comment.
Just move this import to the toplevel, sorted between the rest of the imports.
Sorry, something went wrong.
| # check delay value is not nan | ||
| import math | ||
| if math.isnan(delay): | ||
| raise ValueError("Invalid value NaN (not a number)") |
There was a problem hiding this comment.
Suggestion:
| raise ValueError("Invalid value NaN (not a number)") | |
| raise ValueError("Invalid delay: NaN (not a number)") |
Sorry, something went wrong.
| @@ -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".
Sorry, something went wrong.
|
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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
There was a problem hiding this comment.
You need to add a test for this and versionchanged for docs that nans are now not allowed.
Sorry, something went wrong.
| """Coroutine that completes after a given time (in seconds).""" | ||
|
|
||
| # check delay value is not nan | ||
| if math.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).
Sorry, something went wrong.
…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
There was a problem hiding this comment.
@kumaraditya303 Do you agree?
Sorry, something went wrong.
I pushed some minor tweaks, now LGTM. |
Sorry, something went wrong.
* 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)
Issue: gh-105331:
fix asyncio.sleep(float('nan')) does not raise ValueError