Skip to content

Clarifying nonlocal doc: SyntaxError is raised if nearest enclosing scope is global #114009

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

Merged

Conversation

quazi-irfan
Copy link
Contributor

@quazi-irfan quazi-irfan commented Jan 12, 2024

(Made a new PR because force push messed up the original PR #113572)

N̶o̶n̶l̶o̶c̶a̶l̶ ̶t̶h̶r̶o̶w̶s̶ ̶S̶y̶n̶t̶a̶x̶E̶r̶r̶o̶r̶ ̶i̶f̶ ̶t̶h̶e̶ ̶n̶e̶a̶r̶e̶s̶t̶ ̶e̶n̶c̶l̶o̶s̶i̶n̶g̶ ̶s̶c̶o̶p̶e̶ ̶i̶s̶ ̶g̶l̶o̶b̶a̶l̶.̶ ̶C̶u̶r̶r̶e̶n̶t̶l̶y̶,̶ ̶P̶y̶t̶h̶o̶n̶ ̶L̶a̶n̶g̶u̶a̶g̶e̶ R̶e̶f̶e̶r̶e̶n̶c̶e̶ ̶s̶e̶c̶t̶i̶o̶n̶ ̶E̶x̶e̶c̶u̶t̶i̶o̶n̶ ̶M̶o̶d̶e̶l̶ ̶4̶.̶2̶.̶2̶ ̶d̶o̶e̶s̶ ̶n̶o̶t̶ ̶s̶a̶y̶ ̶s̶o̶.̶ This pull request fixes that. This PR also adds another statement, in the same section, saying global statement creates a variable if existing variable binding is not found.

Here is the link to Python Docs discord discussion thread with @ezio-melotti on this topic.

https://docs.python.org/3/reference/executionmodel.html#resolution-of-names


📚 Documentation preview 📚: https://cpython-previews--114009.org.readthedocs.build/

@AlexWaygood AlexWaygood removed their request for review March 11, 2024 07:36
@JelleZijlstra JelleZijlstra self-requested a review March 14, 2024 14:45
specified in the statement refer to the bindings of those names in the top-level
namespace. Names are resolved in the top-level namespace by searching the
global namespace, i.e. the namespace of the module containing the code block,
global namespace, the namespace of the module containing the code block,
Copy link
Member

Choose a reason for hiding this comment

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

Removing "i.e." makes this wrong: the global namespace is the same as the module namespace here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's what I am implying here. The next part of the sentence is written in the same way.

"the builtins namespace, the namespace of the module builtins" implying the builtins namespace and the namespace of the module builtins are the same.

Copy link
Member

Choose a reason for hiding this comment

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

I see; I don't think that is clear on reading the sentence.

Copy link
Contributor Author

@quazi-irfan quazi-irfan Mar 16, 2024

Choose a reason for hiding this comment

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

How about this version of the sentence?

"Names are resolved in the top-level namespace by searching the global namespace which is the namespace of the module containing the code block, and the builtins namespace which is the namespace of the module builtins."

Doc also uses the word "module namespace" frequently.

Copy link
Member

Choose a reason for hiding this comment

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

That works though I'd put commas before "which". I think the existing wording is fine though, so I'd prefer to keep it.

Copy link
Contributor Author

@quazi-irfan quazi-irfan Mar 16, 2024

Choose a reason for hiding this comment

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

Reverted it.

I think, both of us needing a secondary clarification is a sign that this sentence could be written better.

@JelleZijlstra JelleZijlstra added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Mar 16, 2024
Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

One comment, otherwise LGTM, thanks!

@hugovk hugovk removed the needs backport to 3.11 only security fixes label Apr 11, 2024
@JelleZijlstra JelleZijlstra merged commit 1558d99 into python:main Apr 21, 2024
26 checks passed
@miss-islington-app
Copy link

Thanks @quazi-irfan for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 21, 2024
…cope is global (pythonGH-114009)

(cherry picked from commit 1558d99)

Co-authored-by: Quazi Irfan <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Apr 21, 2024

GH-118128 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Apr 21, 2024
JelleZijlstra added a commit that referenced this pull request Apr 21, 2024
…osing scope is global (GH-114009) (#118128)

Clarifying nonlocal doc: SyntaxError is raised if nearest enclosing scope is global (GH-114009)
(cherry picked from commit 1558d99)

Co-authored-by: Quazi Irfan <[email protected]>
Co-authored-by: Jelle Zijlstra <[email protected]>
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 issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants