Skip to content

/cpython

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-39037: Fix the trial order of the __exit__ and __enter__ methods in the with statement documentation #17608

Merged
merged 8 commits into from Dec 30, 2019
Merged

bpo-39037: Fix the trial order of the __exit__ and __enter__ methods in the with statement documentation #17608

merged 8 commits into from Dec 30, 2019
+49 −18

Conversation

@maggyero
Copy link
Contributor

maggyero commented Dec 14, 2019 •
edited

This PR will make these changes to the Compound statements chapter of the language documentation:

https://bugs.python.org/issue39037
maggyero added 3 commits Dec 14, 2019
@maggyero
Replace the else clause with a finally clause
178721a 
The else clause should instead be a finally clause to also handle the case when there is a non-local goto statement (break, continue, return) in suite (cf. https://stackoverflow.com/questions/59322585/what-is-the-exact-try-statement-equivalent-of-the-with-statement-in-python).
@maggyero
Add the try statement equivalent code of a with statement
0fd6b3e
@maggyero
Fix wrong trial order of __exit__ and __enter__ in the with statement
Loading status checks…
28f27a4
@maggyero maggyero changed the title bpo-39037: Fix wrong trial order of __exit__ and __enter__ in the with statement bpo-39037: Fix the trial order of the __exit__ and __enter__ methods in the with statement documentation Dec 14, 2019
@serhiy-storchaka
serhiy-storchaka reviewed Dec 14, 2019
View changes
Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved
Doc/reference/compound_stmts.rst Outdated Show resolved Hide resolved
Doc/reference/compound_stmts.rst Outdated
@@ -811,23 +834,26 @@ able to suspend execution in its *enter* and *exit* methods.

The following code::

async with EXPR as VAR:
BLOCK
async with expression as target:

This comment has been minimized.

Sign in to view
Copy link
@serhiy-storchaka

serhiy-storchaka Dec 14, 2019

Member

What is the purpose of this change?

This comment has been minimized.

Sign in to view
Copy link
@maggyero

maggyero Dec 14, 2019

Author Contributor

@serhiy-storchaka To use the same terms as in the grammar.

This comment has been minimized.

Sign in to view
Copy link
@brettcannon

brettcannon Dec 16, 2019

Member

It's not important to match the grammar in this case as the docs are not kept in sync with the grammar rule names.

This comment has been minimized.

Sign in to view
Copy link
@ncoghlan

ncoghlan Dec 18, 2019

Contributor

Right, we deliberately use ALL_CAPS in these semantic expansions to distinguish the placeholders from keywords and hidden variables.

This comment has been minimized.

Sign in to view
Copy link
@maggyero

maggyero Dec 18, 2019

Author Contributor

@ncoghlan All right, updated.
Doc/reference/compound_stmts.rst
raise
else:
await aexit(mgr, None, None, None)
finally:

This comment has been minimized.

Sign in to view
Copy link
@serhiy-storchaka

serhiy-storchaka Dec 14, 2019

Member

Why replace else with finally?

This comment has been minimized.

Sign in to view
Copy link
@maggyero

maggyero Dec 14, 2019 •
edited

Author Contributor

@serhiy-storchaka To handle the case when there is a non-local goto statement (break, continue, return) in the suite (cf. https://www.python.org/dev/peps/pep-0343/#specification-the-with-statement). Before knowing about PEP 343 I did the same mistake (cf. https://stackoverflow.com/questions/59322585/what-is-the-exact-try-statement-equivalent-of-the-with-statement-in-python).
@maggyero
Add review suggestions
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
41b99c1
@maggyero maggyero requested a review from serhiy-storchaka Dec 14, 2019
maggyero added 2 commits Dec 15, 2019
@maggyero
Move the target assignment in the try clause
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
d39efb8
@maggyero
Rename a variable
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
be1aa18
@brettcannon brettcannon requested review from brettcannon and ncoghlan Dec 16, 2019
@brettcannon

This comment has been minimized.

Sign in to view
Copy link
Member

brettcannon commented Dec 16, 2019

@maggyero your PR is failing CI.

Adding @ncoghlan as a reviewer since he probably is a good judge about the accuracy of the proposed clarification.
@maggyero maggyero force-pushed the maggyero:patch-13 branch from 5a59435 to 09f882a Dec 16, 2019
@maggyero
Remove unnecessary comments
Loading status checks…
f7b273d
@maggyero maggyero force-pushed the maggyero:patch-13 branch from 09f882a to f7b273d Dec 16, 2019
@maggyero

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

maggyero commented Dec 16, 2019

@brettcannon Fixed, CI passed, thanks.
@brettcannon brettcannon requested review from 1st1 and asvetlov Dec 17, 2019
@maggyero
Use uppercase placeholders
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
1cbc190
@ncoghlan ncoghlan added the skip news label Dec 30, 2019
@ncoghlan
ncoghlan approved these changes Dec 30, 2019
View changes
@ncoghlan ncoghlan merged commit 226e6e7 into python:master Dec 30, 2019
5 checks passed
5 checks passed
Docs
Details
Azure Pipelines PR #20191218.4 succeeded
Details
bedevere/issue-number Issue number 39037 found
Details
bedevere/news "skip news" label found
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Sign in to view
Copy link

miss-islington commented Dec 30, 2019

Thanks @maggyero for the PR, and @ncoghlan for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Dec 30, 2019

GH-17749 is a backport of this pull request to the 3.8 branch.
miss-islington added a commit that referenced this pull request Dec 30, 2019
@miss-islington @maggyero
bpo-39037: Fix lookup order of magic methods in with statement docume…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
cbfafa3 
…ntation (GH-17608)

* __enter__ is now looked up before __exit__ to give a more intuitive error message
* add pseudo-code equivalent for the with statement
* fix pseudo-code for the async with statement to use a finally clause
* use SUITE rather than BLOCK for consistency with the language grammar

Patch by Géry Ogam.
(cherry picked from commit 226e6e7)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncoghlan ncoghlan

@brettcannon brettcannon

@1st1 1st1

@asvetlov asvetlov

@serhiy-storchaka serhiy-storchaka

Assignees
No one assigned
Labels
CLA signed skip news type-documentation
Projects
None yet
Milestone
No milestone
7 participants
@maggyero @brettcannon @miss-islington @bedevere-bot @ncoghlan @serhiy-storchaka @the-knights-who-say-ni
You can’t perform that action at this time.