Skip to content

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

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

Closed
wants to merge 1 commit into from

Conversation

quazi-irfan
Copy link
Contributor

@quazi-irfan quazi-irfan commented Dec 29, 2023

Nonlocal throws SyntaxError if the nearest enclosing scope is global. Currently, Python Language Reference section 4.2.2 does not say so. 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.

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

@ghost
Copy link

ghost commented Dec 29, 2023

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Dec 29, 2023
@@ -1011,7 +1011,9 @@ The :keyword:`nonlocal` statement causes the listed identifiers to refer to
previously bound variables in the nearest enclosing scope excluding globals.
This is important because the default behavior for binding is to search the
local namespace first. The statement allows encapsulated code to rebind
variables outside of the local scope besides the global (module) scope.
variables outside of the local scope besides the global scope. A :exc:`SyntaxError`
will be raised if the nearest enclosing scope is the global (module) scope.
Copy link
Member

Choose a reason for hiding this comment

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

The addition is ok, but I still feel the whole paragraph could be rephrased (possibly in a separate PR), as there are now 4 sentences, 3 of which mentioning the global scope as an exception.

Another option is dealing with the global scope in a separate paragraph, since now the first paragraph describes the default behavior and the following two expand on cases where nonlocal might not work. A separate paragraph could be added to clarify that nonlocal raises a SyntaxError when the nearest enclosing scope is the global scope, and that global can be used instead. Mentions of the global scope could could be then removed from the first paragraph.

Copy link
Contributor Author

@quazi-irfan quazi-irfan Dec 31, 2023

Choose a reason for hiding this comment

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

I've moved this line from Python Language Reference Chapter 7 and moved it to Chapter 4, section 4.2.2 as I found two paragraph highly relevant to the changes you've proposed.

Note: I think branch force push has kind of messed up GH's handling of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with @ezio-melotti ; while the addition does appear to be an overall improvement in clarity, it also makes an already rather klunky and wordy paragraph even more so, with even more repetitive mention of the exception. It would be really nice to fix that broader problem, but maybe that's asking too much of this PR, and would be better done by another author?

I see this change was completely removed...not sure that was what @ezio-melotti was suggesting here?

Note: I think branch force push has kind of messed up GH's handling of this PR.

For this reason, among others, you should avoid force-pushing to (non-draft) PRs where not absolutely necessary, as this makes it difficult or impossible to track the history of a PR and easily review your changes in response to review comments like this. It seems this specific change was entirely removed, but without a commit history it isn't possible to see how or why.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CAM-Gerlach Thanks for the feedback. I don't know how to undo force push, and therefore made a new PR here #114009

@ezio-melotti
I've moved my edits from Ch 7 to Ch 4.2.2 Execution model. What do you think about this change?

@CAM-Gerlach CAM-Gerlach changed the title Clarifying nonlocal doc: SyntaxError is thrown if nearest enclosing scope is global Clarifying nonlocal doc: SyntaxError is raised if nearest enclosing scope is global Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants