Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
gpshead merged 2 commits intopython:mainfromhoodmane:docs-exec-locals
May 20, 2024

Conversation

hoodmane
Copy link
Contributor

@hoodmanehoodmane commentedMay 20, 2024
edited by github-actionsbot
Loading

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 aNameError:

exec("""def f():    print("hi")f()def g():    f()g()""", {}, {})

The reason not to leave out globals is as follows:

deft():exec("""def f():    print("hi")f()def g():    f()g()    """)

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

…ls argumentMany users think they want a locals argument for various reasons but they do notunderstand that it makes code be treated as a class definition. They do not wanttheir code treated as a class definition and get surprised. The reason notto pass locals specifically is that the following code raises a `NameError`:```pyexec("""def f():    print("hi")f()def g():    f()g()""", {}, {})```The reason not to leave out globals is as follows:```pydef t():    exec("""def f():    print("hi")f()def g():    f()g()    """)```
@bedevere-appbedevere-appbot added docsDocumentation in the Doc dir skip news awaiting review labelsMay 20, 2024
@gpsheadgpshead added skip issue needs backport to 3.12only security fixes needs backport to 3.13bugs and security fixes labelsMay 20, 2024
@gpsheadgpshead self-assigned thisMay 20, 2024
same overall meaning.
@gpsheadgpsheadenabled auto-merge (squash)May 20, 2024 17:32
@gpsheadgpshead merged commit7e1a130 intopython:mainMay 20, 2024
@miss-islington-app
Copy link

Thanks@hoodmane for the PR, and@gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 20, 2024
…ls argument (pythonGH-119235)Many users think they want a locals argument for various reasons but they do notunderstand that it makes code be treated as a class definition. They do not wanttheir code treated as a class definition and get surprised. The reason notto pass locals specifically is that the following code raises a `NameError`:```pyexec("""def f():    print("hi")f()def g():    f()g()""", {}, {})```The reason not to leave out globals is as follows:```pydef t():    exec("""def f():    print("hi")f()def g():    f()g()    """)```(cherry picked from commit7e1a130)Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 20, 2024
…ls argument (pythonGH-119235)Many users think they want a locals argument for various reasons but they do notunderstand that it makes code be treated as a class definition. They do not wanttheir code treated as a class definition and get surprised. The reason notto pass locals specifically is that the following code raises a `NameError`:```pyexec("""def f():    print("hi")f()def g():    f()g()""", {}, {})```The reason not to leave out globals is as follows:```pydef t():    exec("""def f():    print("hi")f()def g():    f()g()    """)```(cherry picked from commit7e1a130)Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
@bedevere-app
Copy link

GH-119239 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelMay 20, 2024
@bedevere-app
Copy link

GH-119240 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelMay 20, 2024
gpshead pushed a commit that referenced this pull requestMay 20, 2024
…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 notunderstand that it makes code be treated as a class definition. They do not wanttheir code treated as a class definition and get surprised. The reason notto pass locals specifically is that the following code raises a `NameError`:```pyexec("""def f():    print("hi")f()def g():    f()g()""", {}, {})```The reason not to leave out globals is as follows:```pydef t():    exec("""def f():    print("hi")f()def g():    f()g()    """)```(cherry picked from commit7e1a130)Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
@hoodmanehoodmane deleted the docs-exec-locals branchMay 20, 2024 18:15
gpshead pushed a commit that referenced this pull requestMay 20, 2024
…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 notunderstand that it makes code be treated as a class definition. They do not wanttheir code treated as a class definition and get surprised. The reason notto pass locals specifically is that the following code raises a `NameError`:```pyexec("""def f():    print("hi")f()def g():    f()g()""", {}, {})```The reason not to leave out globals is as follows:```pydef t():    exec("""def f():    print("hi")f()def g():    f()g()    """)```(cherry picked from commit7e1a130)Co-authored-by: Hood Chatham <roberthoodchatham@gmail.com>
@ncoghlan
Copy link
Contributor

ncoghlan commentedMay 21, 2024
edited
Loading

Just a heads up that I included a rewording of this note in the PEP 667 docs PR at#119201 (seehttps://github.com/python/cpython/pull/119201/files#r1607516295 ) after hitting a conflict between this change and that one.

Users pass a separatelocals toexec all the time in order to capture writes, and it only breaks if the code being executed defines and calls functions that attempt to access top-level variables (or includes a class definition that needs to access the class by name). The updated wording both specifies what breaks, and also specifies how to usecollections.ChainMap to capture writes while still running the code with module semantics:

   .. note::      When ``exec`` gets two separate objects as *globals* and *locals*, the      code will be executed as if it were embedded in a class definition. This      means functions and classes defined in the executed code will not be      able to access variables assigned at the top level (as the "top level"      variables are treated as class variables in a class definition).      Passing a :class:`collections.ChainMap` instance as *globals* allows name      lookups to be chained across multiple mappings without triggering this      behaviour. Values assigned to top level names in the executed code can be      retrieved by passing an empty dictionary as the first entry in the chain.

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 and gpshead reacted with thumbs up emoji

@hoodmane
Copy link
ContributorAuthor

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.

ncoghlan reacted with thumbs up emoji

@hoodmane
Copy link
ContributorAuthor

Passing a :class:collections.ChainMap instance asglobals

Interesting!

@ncoghlan
Copy link
Contributor

I actually need to revisit that updated note, as I was reminded today thatglobals has to be a real dict, so passingChainMap doesn't work:

>>> from collections import ChainMap>>> ns = ChainMap({}, globals())>>> exec("print(__name__)", globals())__main__>>> exec("print(__name__)", ns)Traceback (most recent call last):  File "<stdin>", line 1, in <module>TypeError: exec() globals must be a dict, not ChainMap

(and there are unfortunately genuine performance reasons for that restriction, so relaxing it would be easier said than done)

hoodmane reacted with thumbs up emoji

ncoghlan added a commit to ncoghlan/cpython that referenced this pull requestMay 22, 2024
When updating the new exec note added inpythongh-119235 as part of thePEP 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 thenew note to explicitly state that dict merging is the only option.
@ncoghlan
Copy link
Contributor

PR fixing the error:#119378 (and tagged@gpshead for a review)

gpshead reacted with thumbs up emoji

ncoghlan added a commit that referenced this pull requestMay 22, 2024
When updating the new exec note added ingh-119235 as part of thePEP 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.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 22, 2024
When updating the new exec note added inpythongh-119235 as part of thePEP 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 commit31d61a7)Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
ncoghlan added a commit that referenced this pull requestMay 22, 2024
When updating the new exec note added ingh-119235 as part of thePEP 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 commit31d61a7)Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com>
@hoodmane
Copy link
ContributorAuthor

I think what people may want when they pass locals is forglobals to work like a custom overlay overbuiltins andlocals to work likeglobals? I wonder if that is possible.

@gpshead
Copy link
Member

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.

hoodmane reacted with heart emoji

@CNSeniorious000
Copy link

CNSeniorious000 commentedJun 3, 2024
edited
Loading

forglobals to work like a custom overlay overbuiltins andlocals to work likeglobals? I wonder if that is possible.

I think the best practise to implement layered context is to useChainMap.

Let's say you have 2 maps, a Mappingglobal_namespace (acts as the readonly fallback layer, like__builtins__) and a MutableMappinglocal_namespace (acts as the globals). And build a mapnamespace = ChainMap(local_namespace, global_namespace) andgetitem will first trylocal_namespace than tryglobal_namespace, whilesetitem will never modifylocal_namespace.

But! eval'sglobals can only be adict, not aChainMap, so as topyodide.runPython. So you have to use a mixin hack like this:

fromcollectionsimportChainMapclassChainMap(ChainMap,dict):pass

Reproduction:

>>>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 usingexec andeval, so maybe CPython can makeChainMap to inherit fromdict so that we don't need to mixin anymore?

hoodmane reacted with heart emoji

estyxx pushed a commit to estyxx/cpython that referenced this pull requestJul 17, 2024
…ls argument (pythonGH-119235)Many users think they want a locals argument for various reasons but they do notunderstand that it makes code be treated as a class definition. They do not wanttheir code treated as a class definition and get surprised. The reason notto pass locals specifically is that the following code raises a `NameError`:```pyexec("""def f():    print("hi")f()def g():    f()g()""", {}, {})```The reason not to leave out globals is as follows:```pydef t():    exec("""def f():    print("hi")f()def g():    f()g()    """)```
estyxx pushed a commit to estyxx/cpython that referenced this pull requestJul 17, 2024
When updating the new exec note added inpythongh-119235 as part of thePEP 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.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees

@gpsheadgpshead

Labels
docsDocumentation in the Doc dirskip issueskip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@hoodmane@ncoghlan@gpshead@CNSeniorious000

[8]ページ先頭

©2009-2025 Movatter.jp