deprecate *loop* argument for asyncio.sleep

[1st1]

BPO	34728
Nosy	@vstinner, @asvetlov, @serhiy-storchaka, @1st1, @willingc, @tirkarthi, @fbidu
PRs	bpo-34728: Remove deprecate *loop* argument in asyncio.sleep #9415 bpo-34728: Fix asyncio tests to run under "-Werror" #9661

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-10-02.17:59:06.584>
created_at = <Date 2018-09-18.21:44:05.146>
labels = ['easy', 'type-feature', '3.8', 'expert-asyncio']
title = 'deprecate *loop* argument for asyncio.sleep'
updated_at = <Date 2018-10-02.17:59:06.583>
user = 'https://github.com/1st1'

bugs.python.org fields:

activity = <Date 2018-10-02.17:59:06.583>
actor = 'yselivanov'
assignee = 'none'
closed = True
closed_date = <Date 2018-10-02.17:59:06.584>
closer = 'yselivanov'
components = ['asyncio']
creation = <Date 2018-09-18.21:44:05.146>
creator = 'yselivanov'
dependencies = []
files = []
hgrepos = []
issue_num = 34728
keywords = ['patch', 'easy']
message_count = 12.0
messages = ['325684', '325685', '326213', '326825', '326826', '326832', '326862', '326863', '326864', '326866', '326890', '326891']
nosy_count = 7.0
nosy_names = ['vstinner', 'asvetlov', 'serhiy.storchaka', 'yselivanov', 'willingc', 'xtreak', 'fbidu']
pr_nums = ['9415', '9661']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue34728'
versions = ['Python 3.8']

[1st1]

asyncio.sleep is a coroutine; passing a *loop* argument to it makes no sense anymore.

We should raise a DeprecationWarning in Python 3.8 and 3.9 and remove it in 4.0.

[1st1]

Same for asyncio.wait and asyncio.wait_for.

[willingc]

New changeset 558c49b by Carol Willing (João Júnior) in branch 'master':
bpo-34728: Remove deprecate *loop* argument in asyncio.sleep (GH-9415)
558c49b

[serhiy-storchaka]

Tests are failed when ran with -Werror.

$ ./python -Werror -m test -vuall test_asyncgen
...

======================================================================
ERROR: test_async_gen_asyncio_01 (test.test_asyncgen.AsyncGenAsyncioTest)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_asyncgen.py", line 404, in test_async_gen_asyncio_01
    res = self.loop.run_until_complete(self.to_list(gen()))
  File "/home/serhiy/py/cpython/Lib/asyncio/base_events.py", line 582, in run_until_complete
    return future.result()
  File "/home/serhiy/py/cpython/Lib/test/test_asyncgen.py", line 391, in to_list
    async for i in gen:
  File "/home/serhiy/py/cpython/Lib/test/test_asyncgen.py", line 398, in gen
    await asyncio.sleep(0.01, loop=self.loop)
  File "/home/serhiy/py/cpython/Lib/asyncio/tasks.py", line 598, in sleep
    warnings.warn("The loop argument is deprecated and scheduled for "
DeprecationWarning: The loop argument is deprecated and scheduled for removal in Python 3.10.

======================================================================
...
(the full log is too long)

$ ./python -Werror -m test -vuall test_asyncio
...

======================================================================
FAIL: test_sleep_cancel (test.test_asyncio.test_tasks.PyTask_PyFuture_SubclassTests)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_asyncio/utils.py", line 510, in close_loop
    loop.close()
  File "/home/serhiy/py/cpython/Lib/test/test_asyncio/utils.py", line 362, in close
    self._gen.send(0)
  File "/home/serhiy/py/cpython/Lib/test/test_asyncio/test_tasks.py", line 1363, in gen
    self.assertAlmostEqual(10.0, when)
AssertionError: 10.0 != 0 within 7 places (10.0 difference)

======================================================================
FAIL: test_run_coroutine_threadsafe_task_factory_exception (test.test_asyncio.test_tasks.RunCoroutineThreadsafeTests)
Test coroutine submission from a tread to an event loop
----------------------------------------------------------------------

Traceback (most recent call last):
  File "/home/serhiy/py/cpython/Lib/test/test_asyncio/test_tasks.py", line 3189, in test_run_coroutine_threadsafe_task_factory_exception
    self.assertEqual(len(callback.call_args_list), 1)
AssertionError: 2 != 1

---

...
(the full log is too long)

[vstinner]

We should raise a DeprecationWarning in Python 3.8 and 3.9 and remove it in 4.0.

On python-committers, it has been said that 3.10 will follow Python 3.9, no?

[1st1]

On python-committers, it has been said that 3.10 will follow Python 3.9, no?

Victor, you're looking at an outdated comment on this issue. This is what's in the master branch:

cpython/Lib/asyncio/tasks.py

Lines 598 to 599 in d4c76d9

	warnings.warn("The loop argument is deprecated and scheduled for "
	"removal in Python 3.10.",

[vstinner]

Victor, you're looking at an outdated comment on this issue.

No, I read the commit:

558c49b

        warnings.warn("The loop argument is deprecated and scheduled for"
                      "removal in Python 4.0.",
                      DeprecationWarning, stacklevel=2)

This is what's in the master branch:

cpython/Lib/asyncio/tasks.py

Lines 598 to 599 in d4c76d9

	warnings.warn("The loop argument is deprecated and scheduled for "
	"removal in Python 3.10.",

Ah, you forgot to mention this bpo in your commit:

commit fad6af2
Author: Yury Selivanov <yury@magic.io>
Date: Tue Sep 25 17:44:52 2018 -0400

asyncio/docs: Replace Python 4.0 -> 3.10 (GH-9579)

Anyway, the current code is fine. Thanks.

[vstinner]

There is a new PR, so I change the issue resolution again.

[vstinner]

FYI if I recall correctly, in the past, we preferred to pass explicitly the loop to avoid to have to get the current loop which may add an overhead. But the current trend is to get rid of the explicit loop parameter.

asyncio.sleep is a coroutine; passing a *loop* argument to it makes no sense anymore.

sleep() requires the current event loop:

    if loop is None:
        loop = events.get_running_loop()
    else:
        warnings.warn("The loop argument is deprecated and scheduled for "
                      "removal in Python 3.10.",
                      DeprecationWarning, stacklevel=2)

    future = loop.create_future()
    h = loop.call_later(delay,
                        futures._set_result_unless_cancelled,
                        future, result)

Why does it not make sense to pass the loop to sleep? "it makes no sense anymore" something changes?

I'm not against the change, I'm just trying to understand the rationale for other changes :-)

[asvetlov]

My understanding is:

loop argument passed to sleep should be always the same as returned from get_running_loop().

Passing it explicitly can be considered as microoptimization but get_running_loop() is pretty fast now, no need for such micro-opts.

On another hand passing *non-current* loop is a serious error: nothing prevents to do it but the code just hangs.

[1st1]

New changeset 9012a0f by Yury Selivanov in branch 'master':
bpo-34728: Fix asyncio tests to run under "-Werror" (GH-9661)
9012a0f

[1st1]

[victor]

Why does it not make sense to pass the loop to sleep? "it makes no sense anymore" something changes?

[andrew]
loop argument passed to sleep should be always the same as returned from get_running_loop().

What Andrew said.

Basically, it wasn't *ever* possible to pass a loop to sleep() that would be different from the loop that would run it, because sleep() is a *coroutine*.

In asyncio some APIs are functions and some are coroutines.

• asyncio.gather(), for example, is a function. You can call it from top-level code (and pass an event loop to it) or from a coroutine.

• asyncio.sleep(), wait(), and wait_for() are *coroutines*; they can only be called from other coroutines or if you wrap them into a Task; in all cases, *loop* is always 100% defined for them.

Passing the loop isn't even a viable micro-optimization, it's just pointless. This extra argument just adds to the confusion and promotes bad patterns, so we want to eventually remove it.