Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
DOCS: Suggest always calling exec with a globals argument and no locals argument#119235
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
hoodmane commented May 20, 2024 • edited by github-actions bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by github-actions bot
Uh oh!
There was an error while loading. Please reload this page.
…ls argument Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a `NameError`: ```py exec(""" def f(): print("hi") f() def g(): f() g() """,{},{}) ``` The reason not to leave out globals is as follows: ```py def t(): exec(""" def f(): print("hi") f() def g(): f() g() """) ```same overall meaning.
…ls argument (pythonGH-119235) Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a `NameError`: ```py exec(""" def f(): print("hi") f() def g(): f() g() """,{},{}) ``` The reason not to leave out globals is as follows: ```py def t(): exec(""" def f(): print("hi") f() def g(): f() g() """) ``` (cherry picked from commit 7e1a130) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
…ls argument (pythonGH-119235) Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a `NameError`: ```py exec(""" def f(): print("hi") f() def g(): f() g() """,{},{}) ``` The reason not to leave out globals is as follows: ```py def t(): exec(""" def f(): print("hi") f() def g(): f() g() """) ``` (cherry picked from commit 7e1a130) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
GH-119239 is a backport of this pull request to the 3.13 branch. |
GH-119240 is a backport of this pull request to the 3.12 branch. |
…no locals argument (GH-119235) (#119240) DOCS: Suggest always calling exec with a globals argument and no locals argument (GH-119235) Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a `NameError`: ```py exec(""" def f(): print("hi") f() def g(): f() g() """,{},{}) ``` The reason not to leave out globals is as follows: ```py def t(): exec(""" def f(): print("hi") f() def g(): f() g() """) ``` (cherry picked from commit 7e1a130) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
…no locals argument (GH-119235) (#119239) DOCS: Suggest always calling exec with a globals argument and no locals argument (GH-119235) Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a `NameError`: ```py exec(""" def f(): print("hi") f() def g(): f() g() """,{},{}) ``` The reason not to leave out globals is as follows: ```py def t(): exec(""" def f(): print("hi") f() def g(): f() g() """) ``` (cherry picked from commit 7e1a130) Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
ncoghlan commented May 21, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Just a heads up that I included a rewording of this note in the PEP 667 docs PR at #119201 (see https://github.com/python/cpython/pull/119201/files#r1607516295 ) after hitting a conflict between this change and that one. Users pass a separate That PR will only get backported as far as 3.13 (since the rest of the PR is specific to a 3.13 change). I'm not sure it's worth worrying about changing the note in 3.12, though. |
hoodmane commented May 21, 2024
Thanks @ncoghlan! I wanted to keep to as few words as possible here so that the PR wouldn't get bogged down in discussion. I am always concerned that each added word makes it more likely that the PR leads to an endless discussion, I run out of energy, and it goes stale. |
hoodmane commented May 21, 2024
Interesting! |
ncoghlan commented May 22, 2024
I actually need to revisit that updated note, as I was reminded today that (and there are unfortunately genuine performance reasons for that restriction, so relaxing it would be easier said than done) |
When updating the new exec note added in pythongh-119235 as part of the PEP 667 general docs PR, I suggested a workaround that isn't valid. Since it's far from the first time I've considered that workaround, and the fact it doesn't work has surprised me every time, amend the new note to explicitly state that dict merging is the only option.
ncoghlan commented May 22, 2024
When updating the new exec note added in gh-119235 as part of the PEP 667 general docs PR, I suggested a workaround that isn't valid. The first half of the note is still reasonable, so just omit the invalid text.
When updating the new exec note added in pythongh-119235 as part of the PEP 667 general docs PR, I suggested a workaround that isn't valid. The first half of the note is still reasonable, so just omit the invalid text. (cherry picked from commit 31d61a7) Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
When updating the new exec note added in gh-119235 as part of the PEP 667 general docs PR, I suggested a workaround that isn't valid. The first half of the note is still reasonable, so just omit the invalid text. (cherry picked from commit 31d61a7) Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
hoodmane commented May 22, 2024
I think what people may want when they pass locals is for |
gpshead commented May 22, 2024
Thanks Alyssa, I wasn't totally happy with the wording in this PR but felt it started to shift us towards the right thing to express. I like your wording better. On of my key issues is that long existing "the code will be executed as if it were embedded in a class definition" wording (from before this PR) is the kind of thing that most docs readers are not likely to understand as it's "weird" but most people need never understand it. Your new text expands upon that in an accessible descriptive manner. |
CNSeniorious000 commented Jun 3, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I think the best practise to implement layered context is to use Let's say you have 2 maps, a Mapping But! eval's fromcollectionsimportChainMapclassChainMap(ChainMap, dict): passReproduction:>>>fromcollectionsimportChainMap>>>g, l={"a": 2},{} >>>source="""... def f():... return g() + 1...... def g():... return a...... a = f()... """>>>exec(source, ChainMap(l, g)) Traceback (mostrecentcalllast): File"<console>", line1, in<module>TypeError: globalsmustbearealdict; tryeval(expr,{}, mapping) >>>classChainMap(ChainMap, dict): ... pass ... >>>exec(source, ChainMap(l, g)) >>>g{'a': 2} >>>l{'f': <functionfat0xa04368>, 'g': <functiongat0x12654d0>, 'a': 3}I think its a common use case to avoid pollution on the global namespace when using |
…ls argument (pythonGH-119235) Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a `NameError`: ```py exec(""" def f(): print("hi") f() def g(): f() g() """,{},{}) ``` The reason not to leave out globals is as follows: ```py def t(): exec(""" def f(): print("hi") f() def g(): f() g() """) ```
When updating the new exec note added in pythongh-119235 as part of the PEP 667 general docs PR, I suggested a workaround that isn't valid. The first half of the note is still reasonable, so just omit the invalid text.
Many users think they want a locals argument for various reasons but they do not understand that it makes code be treated as a class definition. They do not want their code treated as a class definition and get surprised. The reason not to pass locals specifically is that the following code raises a
NameError:The reason not to leave out globals is as follows:
📚 Documentation preview 📚: https://cpython-previews--119235.org.readthedocs.build/