FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-41879: Doc: Fix description of async for statement by nickgaya · Pull Request #23548 · python/cpython · GitHub

Repository navigation

bpo-41879: Doc: Fix description of async for statement - #23548

Merged
miss-islington merged 4 commits into
python:masterfrom
nickgaya:async-for-doc-fix
Dec 11, 2020
Merged

miss-islington merged 4 commits into
python:masterfrom
nickgaya:async-for-doc-fix

Conversation

nickgaya commented Nov 29, 2020 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Contributor

Fix the wording in the documentation of async for to correctly describe asynchronous iterables. This fix is relevant for version 3.7 onward.

https://bugs.python.org/issue41879

Automerge-Triggered-By: GH:aeros

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@nickgaya

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Nov 29, 2020
Comment thread Doc/reference/compound_stmts.rst Outdated
An :term:`asynchronous iterable` is able to call asynchronous code in its
*iter* implementation, and :term:`asynchronous iterator` can call asynchronous
code in its *next* method.
An :term:`asynchronous iterable` provides an *iter* method that returns an

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Technically *iter* is __aiter__ and *next* is __anext__.

I have no idea how to write the text better. Maybe it should not name iter/next at all but define async-iterable/async-iterator in plain English words?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I agree it's a bit unclear, but consistent with how async context managers are described in the async with section.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I believe async for description should be compatible with for, async with should follow with structure.

@1st1 @aeros please share your opinions.

JulienPalard Dec 1, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Beware english not my primary language, but here's my proposed wording:

An :term:`asynchronous iterable` provides an ``__aiter__`` method that directly returns an
:term:`asynchronous iterator`, which can call asynchronous code in its ``async __anext__``
method.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good.

@1st1 @aeros maybe you can review the proposal?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like Julien's phrasing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+1 from me to go with Julian's proposal.

nickgaya Dec 7, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not too keen on this change as I think the reason for using italicized words originally was to refer to the methods without getting into the specifics, which are described in more detail in the data model documentation for asynchronous iterators. Compare to the async with section, which introduces the concept of an asynchronous context manager as "a context manager that is able to suspend execution in its enter and exit methods" rather than using the exact names.

However Julien's proposal seems to have majority support so I updated the PR accordingly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The reference section of the docs is typically intended to be more technically accurate, rather than prioritizing a simplified introduction. I wouldn't be opposed to applying the same to async with (to instead refer to __aexit__ and __aenter__), but don't think it's worth changing by itself.

aeros commented Dec 11, 2020

Copy link
Copy Markdown
Contributor

By the way, thanks for the PR and welcome @nickgaya!

aeros commented Dec 11, 2020

Copy link
Copy Markdown
Contributor

Adding backport label only for 3.9, as it's the latest version present on docs.python.org. I typically don't backport lower than that for documentation PRs, since there's minimal to no benefit for most users.

miss-islington merged commit 4b8cdfc into python:master Dec 11, 2020

Copy link
Copy Markdown
Contributor

Thanks @nickgaya for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

Copy link
Copy Markdown
Contributor

Thanks @nickgaya for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

Copy link
Copy Markdown
Contributor

Thanks @nickgaya for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

Copy link
Copy Markdown

GH-23749 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 13, 2020
Fix the wording in the documentation of `async for` to correctly describe asynchronous iterables.  This fix is relevant for version 3.7 onward.
(cherry picked from commit 4b8cdfc)

Co-authored-by: Nick Gaya <nicholasgaya+github@gmail.com>
miss-islington added a commit that referenced this pull request Dec 13, 2020
… (GH-23749)

Fix the wording in the documentation of `async for` to correctly describe asynchronous iterables.  This fix is relevant for version 3.7 onward.
(cherry picked from commit 4b8cdfc)


Co-authored-by: Nick Gaya <nicholasgaya+github@gmail.com>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Fix the wording in the documentation of `async for` to correctly describe asynchronous iterables.  This fix is relevant for version 3.7 onward.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants


Back | FazBrowse Home | New Git URL