classification
Title: asyncio.wait function creates futures set two times
Type: enhancement Stage: resolved
Components: asyncio Versions: Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: JustinTArthur, asvetlov, chris.jerdonek, dutradda, jstasiak, miss-islington, ods, yselivanov
Priority: normal Keywords: patch

Created on 2020-10-24 06:51 by dutradda, last changed 2020-11-11 13:28 by asvetlov. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22938 merged dutradda, 2020-10-24 06:51
PR 23233 merged miss-islington, 2020-11-10 22:13
Messages (13)
msg379522 - (view) Author: Diogo Dutra (dutradda) * Date: 2020-10-24 06:51
The python3.9 code creates the futures set two times.
We can create this set before, avoiding the second creation.

This python3.9 behaviour breaks the aiokafka library, because it gives an iterator to that function, so the second iteration become empty.
msg379613 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-10-25 21:57
Are you suggesting this is a bug, or is it just a suggested code cleanup? I ask because the docs suggest that a set should be passed:
https://docs.python.org/3/library/asyncio-task.html#asyncio.wait

And the docstring says it should be a sequence:
https://github.com/python/cpython/blob/d1a0a960ee493262fb95a0f5b795b5b6d75cecb8/Lib/asyncio/tasks.py#L373-L376

So while code cleanup is okay, I'm not sure support for general iterator arguments can / should be guaranteed.
msg379638 - (view) Author: Denis S. Otkidach (ods) Date: 2020-10-26 04:32
The current error message is way too cryptic anyway.  And restricting it to the set type only, as in docs, will certainly break a lot of code (passing a list is quite common).
msg379685 - (view) Author: Diogo Dutra (dutradda) * Date: 2020-10-26 20:28
> Are you suggesting this is a bug, or is it just a suggested code cleanup?

It is a suggested code cleanup.

My point is that the code creates two sets based on the Sequence `fs`.
I think it is better if the code creates the set just one time, instead of
two times.

Em dom., 25 de out. de 2020 às 18:57, Chris Jerdonek <report@bugs.python.org>
escreveu:

>
> Chris Jerdonek <chris.jerdonek@gmail.com> added the comment:
>
> Are you suggesting this is a bug, or is it just a suggested code cleanup?
> I ask because the docs suggest that a set should be passed:
> https://docs.python.org/3/library/asyncio-task.html#asyncio.wait
>
> And the docstring says it should be a sequence:
>
> https://github.com/python/cpython/blob/d1a0a960ee493262fb95a0f5b795b5b6d75cecb8/Lib/asyncio/tasks.py#L373-L376
>
> So while code cleanup is okay, I'm not sure support for general iterator
> arguments can / should be guaranteed.
>
> ----------
> nosy: +chris.jerdonek
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue42140>
> _______________________________________
>
msg379689 - (view) Author: Justin Arthur (JustinTArthur) * Date: 2020-10-26 21:30
Your change makes perfect sense to me. It would be a backport-only change as the 2nd set creation is actually getting removed for the development version (3.10) to finalize the deprecation of wait's coroutine scheduling.
msg379714 - (view) Author: Chris Jerdonek (chris.jerdonek) * (Python committer) Date: 2020-10-27 02:29
If it's just a code cleanup and not a bugfix though, it shouldn't be backported. But yes, as a cleanup it's fine. And even if it's expected to go away later, it's okay to do now.
msg379725 - (view) Author: Justin Arthur (JustinTArthur) * Date: 2020-10-27 03:24
I believe the documentation may be referring to the English set and not a Python set, but I could be wrong.

Yury changed the wording from sequence to set in 3.7, but we didn't document a breaking change as far as I know. The purpose of those set constructions was to handle the many things other than a set that can be passed in that may have non-unique values. Early asyncio documentation included examples of passing wait() a list.

I wouldn't be surprised if there are other libraries or apps out there for which removing iterator support was an accidentally-breaking change and it may be strange if this is only is broken in 3.9.x, but working both before and after.
msg379734 - (view) Author: Diogo Dutra (dutradda) * Date: 2020-10-27 04:21
> I wouldn't be surprised if there are other libraries or apps out there
for which removing iterator support was an accidentally-breaking change and
it may be strange if this is only is broken in 3.9.x, but working both
before and after.

Justin, when the deprecation message has been removed, the iterator will
works again, because the code will iterate just one time.
So the behaviour you describe will happen anyway.

My changes proposal is a “improvement”, because the second iteration can be
made on the already created set.

I think that change make senses because it removes unnecessary data
creation.

Em ter, 27 de out de 2020 às 00:24, Justin Arthur <report@bugs.python.org>
escreveu:

>
> Justin Arthur <justinarthur@gmail.com> added the comment:
>
> I believe the documentation may be referring to the English set and not a
> Python set, but I could be wrong.
>
> Yury changed the wording from sequence to set in 3.7, but we didn't
> document a breaking change as far as I know. The purpose of those set
> constructions was to handle the many things other than a set that can be
> passed in that may have non-unique values. Early asyncio documentation
> included examples of passing wait() a list.
>
> I wouldn't be surprised if there are other libraries or apps out there for
> which removing iterator support was an accidentally-breaking change and it
> may be strange if this is only is broken in 3.9.x, but working both before
> and after.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue42140>
> _______________________________________
>
msg379736 - (view) Author: Justin Arthur (JustinTArthur) * Date: 2020-10-27 04:57
> So the behaviour you describe will happen anyway.
Unless we make your change against the 3.9 branch, in which case the fix will make it into the 3.9.x series of patch releases.
msg380129 - (view) Author: Jakub Stasiak (jstasiak) * Date: 2020-11-01 12:26
I opened https://bugs.python.org/issue42230 to make the documentation reflect the reality in this matter.
msg380721 - (view) Author: miss-islington (miss-islington) Date: 2020-11-10 22:13
New changeset 7e5ef0a5713f968f6e942566c78bf57ffbef01de by Diogo Dutra in branch 'master':
bpo-42140: Improve asyncio.wait function (GH-22938)
https://github.com/python/cpython/commit/7e5ef0a5713f968f6e942566c78bf57ffbef01de
msg380726 - (view) Author: miss-islington (miss-islington) Date: 2020-11-10 23:11
New changeset 33922cb0aa0c81ebff91ab4e938a58dfec2acf19 by Miss Islington (bot) in branch '3.9':
bpo-42140: Improve asyncio.wait function (GH-22938)
https://github.com/python/cpython/commit/33922cb0aa0c81ebff91ab4e938a58dfec2acf19
msg380727 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2020-11-10 23:12
Thanks for the change and for the discussion to everybody involved. The PR makes sense and I've already merged it.
History
Date User Action Args
2020-11-11 13:28:36asvetlovsetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.10
2020-11-10 23:12:32yselivanovsetmessages: + msg380727
2020-11-10 23:11:04miss-islingtonsetmessages: + msg380726
2020-11-10 22:13:22miss-islingtonsetkeywords: + patch
stage: patch review
pull_requests: + pull_request22129
2020-11-10 22:13:01miss-islingtonsetnosy: + miss-islington
messages: + msg380721
2020-11-01 12:26:47jstasiaksetmessages: + msg380129
2020-11-01 10:04:46jstasiaksetnosy: + jstasiak
2020-10-27 04:57:25JustinTArthursetmessages: + msg379736
2020-10-27 04:21:19dutraddasetmessages: + msg379734
2020-10-27 03:24:53JustinTArthursetmessages: + msg379725
2020-10-27 02:29:32chris.jerdoneksetmessages: + msg379714
2020-10-26 21:30:35JustinTArthursetnosy: + JustinTArthur
messages: + msg379689
2020-10-26 20:28:26dutraddasetmessages: + msg379685
2020-10-26 04:32:54odssetmessages: + msg379638
2020-10-25 21:57:48chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg379613
2020-10-24 07:08:49odssetnosy: + ods
2020-10-24 06:51:22dutraddacreate