gh-105331: Fix asyncio.sleep() bug #105641
Conversation
weijay0804
requested review from
1st1,
asvetlov,
gvanrossum,
kumaraditya303 and
willingc
as code owners
|
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
| @@ -950,21 +950,21 @@ def callback(): | |||
| def create_eager_task_factory(custom_task_constructor): | |||
| """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
| @@ -642,6 +642,12 @@ def __sleep0(): | |||
|
|
|||
| async def sleep(delay, result=None): | |||
| """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.
Lib/asyncio/tasks.py
Outdated
| # 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)") |
| @@ -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".
|
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 |
weijay0804
force-pushed
the
fix-issue-105331
branch
from
b3eed28 to
408814d
Compare
weijay0804
force-pushed
the
fix-issue-105331
branch
from
c0fa428 to
9bd7ced
Compare
Lib/asyncio/tasks.py
Outdated
| @@ -642,6 +643,11 @@ def __sleep0(): | |||
|
|
|||
| async def sleep(delay, result=None): | |||
| """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).
Misc/NEWS.d/next/Core and Builtins/2023-06-11-09-14-30.gh-issue-105331.nlZvoW.rst
Outdated
Show resolved
Hide resolved
…e-105331.nlZvoW.rst I think this way is more clearer. Thank you! Co-authored-by: Guido van Rossum <gvanrossum@gmail.com>
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)
Issue: gh-105331:
fix asyncio.sleep(float('nan')) does not raise ValueError