New submission from Bar Harel <bzvi7919@gmail.com>: Continuing with bpo-27589, looks like as_completed documentation is still misleading. According to the docs, it "Return(s) an iterator of Future objects. Each Future object returned represents the earliest result from the set of the remaining awaitables." There's only one problem: The only thing it definitely doesn't do, is return an iterator of future objects. To be honest with you, I'm not entirely sure how to phrase it. For reference, I fell for this: mapping = {fut: index for fut, index in enumerate(futures)} for fut in as_completed(mapping): mapping[fut] # KeyError ---------- assignee: docs@python components: Documentation, asyncio messages: 367410 nosy: aronacher, asvetlov, bar.harel, docs@python, gvanrossum, hynek, vstinner, xtreak, yselivanov priority: normal severity: normal status: open title: ast versions: Python 3.7, Python 3.8, Python 3.9 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Change by Bar Harel <bzvi7919@gmail.com>: ---------- title: ast -> asyncio.as_completed documentation misleading _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Change by STINNER Victor <vstinner@python.org>: ---------- nosy: -vstinner _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Guido van Rossum <guido@python.org> added the comment: I declare this not a bug. The docs do not promise that the Futures being returned are the *same* Futures that were passed in. They are not. They are (or at least may be) new Futures that represent the same event. Since Futures, when used as dict keys, use identity as equality, those new Futures will not be present as keys in the mapping of Futures passed in by the OP. ---------- resolution: -> not a bug stage: -> resolved status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Guido van Rossum <guido@python.org> added the comment: Reopening so as to give the OP one more chance to state their case. They wrote: """ You've immediately closed the issue so I couldn't even reply to it, Unfortunately, it doesn't return a Future object at all, so technically you're wrong, together with the docs of course, which was the bug I reported... """ If it's not a Future then what? You're not showing that in your report. ---------- status: closed -> open _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Bar Harel <bzvi7919@gmail.com> added the comment: It's a coroutine. Basically the same coroutine yielded over and over, returning the first future's result each time. Like I said, I'm not entirely sure how to phrase it. Maybe "Returns an iterator of awaitables"? ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Guido van Rossum <guido@python.org> added the comment: Oh, you're right. The docstring correctly says this. :-( Do you have the power to submit a PR? I think it should just say that the return values are coroutines (which is what it does). @Yury what do you think? ---------- resolution: not a bug -> _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Yury Selivanov <yselivanov@gmail.com> added the comment:
@Yury what do you think?
Yeah, the documentation needs to be fixed.
Maybe "Returns an iterator of awaitables"?
I'd suggest to change to: "Return an iterator of coroutines. Each coroutine allows to wait for the earliest next result from the set of the remaining awaitables." ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Change by Bar Harel <bzvi7919@gmail.com>: ---------- keywords: +patch pull_requests: +19074 stage: resolved -> patch review pull_request: https://github.com/python/cpython/pull/19753 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Kyle Stanley <aeros167@gmail.com> added the comment: Yury Selivanov wrote:
I'd suggest to change to: "Return an iterator of coroutines. Each coroutine allows to wait for the earliest next result from the set of the remaining awaitables."
The first sentence looks good to me, but I'm not certain about the second sentence, particularly "allows to wait". Instead, I'm thinking something like this might read better: "Each coroutine represents the earliest next result from the set of the remaining awaitables, based upon the order of completion." Thoughts? ---------- nosy: +aeros _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Kyle Stanley <aeros167@gmail.com> added the comment:
based upon the order of completion
I forgot to explain this in the above message. I think something that mentions "order of completion" should be included. Although it's implied in the name `as_completed`, to me it seems worthwhile to be fully explicit with what exactly is meant by "earliest". ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Kyle Stanley <aeros167@gmail.com> added the comment: Alternatively, I think "can wait for" would also read better than "allows to wait for", if that is preferred over my above suggestion. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Guido van Rossum <guido@python.org> added the comment: Please work this out in the PR between the two of you, Kyle and Bar. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Bar Harel <bzvi7919@gmail.com> added the comment: @Kyle, looks good to me. Maybe this will work better? Together with the current example which shows the "earliest_result" variable of course. "Each coroutine returns the next result from the set of the remaining awaitables, based upon the order of completion." Represents -> Returns (less obscure, and coroutines are awaited on to get the result) Earliest -> removed, "order of completion" looks great. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Kyle Stanley <aeros167@gmail.com> added the comment: @Bar as requested by Guido, let's continue the discussion in the PR: https://github.com/python/cpython/pull/19753#pullrequestreview-403185142. ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Kyle Stanley <aeros167@gmail.com> added the comment: New changeset 13206b52d16c2489f4c7dd2dce2a7f48a554b5ed by Bar Harel in branch 'master': bpo-40405: Fix asyncio.as_completed docs (GH-19753) https://github.com/python/cpython/commit/13206b52d16c2489f4c7dd2dce2a7f48a55... ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Change by miss-islington <mariatta.wijaya+miss-islington@gmail.com>: ---------- nosy: +miss-islington nosy_count: 9.0 -> 10.0 pull_requests: +19605 pull_request: https://github.com/python/cpython/pull/20337 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Change by miss-islington <mariatta.wijaya+miss-islington@gmail.com>: ---------- pull_requests: +19606 pull_request: https://github.com/python/cpython/pull/20338 _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
miss-islington <mariatta.wijaya+miss-islington@gmail.com> added the comment: New changeset 9181e2e2f3ade85f65bf2521a5deb898434dfd64 by Miss Islington (bot) in branch '3.9': bpo-40405: Fix asyncio.as_completed docs (GH-19753) https://github.com/python/cpython/commit/9181e2e2f3ade85f65bf2521a5deb898434... ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
miss-islington <mariatta.wijaya+miss-islington@gmail.com> added the comment: New changeset 2fecb48a1d84190c37214eb4b0c8d5460300a78b by Miss Islington (bot) in branch '3.8': bpo-40405: Fix asyncio.as_completed docs (GH-19753) https://github.com/python/cpython/commit/2fecb48a1d84190c37214eb4b0c8d546030... ---------- _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
Kyle Stanley <aeros167@gmail.com> added the comment: Thanks for bring this to our attention and working on this Bar Harel, and thanks to Yury for the suggestions. ---------- resolution: -> fixed stage: patch review -> resolved status: open -> closed _______________________________________ Python tracker <report@bugs.python.org> <https://bugs.python.org/issue40405> _______________________________________
participants (6)
-
Bar Harel -
Guido van Rossum
-
Kyle Stanley
-
miss-islington
-
STINNER Victor
-
Yury Selivanov