GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — DOCS: Suggest always calling exec with a globals argument and no locals argument by hoodmane · Pull Request #119235 · python/cpython

Conversation

@hoodmane

@hoodmane hoodmane commented May 20, 2024 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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:

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

f()

def g():
    f()
g()
""", {}, {})

The reason not to leave out globals is as follows:

def t():
    exec("""
def f():
    print("hi")

f()

def g():
    f()
g()
    """)

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@hoodmane
DOCS: Suggest always calling exec with a globals argument and no loca…
0f09e03
…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()
    """)
```
@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir skip news awaiting review labels May 20, 2024
@gpshead gpshead added skip issue needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels May 20, 2024
@gpshead gpshead self-assigned this May 20, 2024
@gpshead
Tweak the wording
39a3bf7
same overall meaning.
@gpshead
gpshead enabled auto-merge (squash) May 20, 2024 17:32
@gpshead
gpshead merged commit 7e1a130 into python:main May 20, 2024
@miss-islington-app

miss-islington-app Bot commented May 20, 2024

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting review label May 20, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 20, 2024
@hoodmane @miss-islington
DOCS: Suggest always calling exec with a globals argument and no loca…
9b325a5
…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 <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 20, 2024
@hoodmane @miss-islington
DOCS: Suggest always calling exec with a globals argument and no loca…
51ec70d
…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 <[email protected]>
@bedevere-app

bedevere-app Bot commented May 20, 2024

Copy link
Copy Markdown

GH-119239 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label May 20, 2024
@bedevere-app

bedevere-app Bot commented May 20, 2024

Copy link
Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.12 only security fixes label May 20, 2024
gpshead pushed a commit that referenced this pull request May 20, 2024
@miss-islington @hoodmane
[3.12] DOCS: Suggest always calling exec with a globals argument and …
a50f065
…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 <[email protected]>
@hoodmane
hoodmane deleted the docs-exec-locals branch May 20, 2024 18:15
gpshead pushed a commit that referenced this pull request May 20, 2024
@miss-islington @hoodmane
[3.13] DOCS: Suggest always calling exec with a globals argument and …
fda3291
…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 <[email protected]>
@ncoghlan

ncoghlan commented May 21, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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 locals to exec 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 use collections.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
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ncoghlan ncoghlan mentioned this pull request May 21, 2024
gh-74929: PEP 667 general docs update #119201
Merged
@hoodmane

hoodmane commented May 21, 2024

Copy link
Copy Markdown
Contributor Author

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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@hoodmane

hoodmane commented May 21, 2024

Copy link
Copy Markdown
Contributor Author

Passing a :class:collections.ChainMap instance as globals

Interesting!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ncoghlan

ncoghlan commented May 22, 2024

Copy link
Copy Markdown
Contributor

I actually need to revisit that updated note, as I was reminded today that globals has to be a real dict, so passing ChainMap 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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ncoghlan added a commit to ncoghlan/cpython that referenced this pull request May 22, 2024
@ncoghlan
DOCS: fix error in exec namespace note
14b86be
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 ncoghlan mentioned this pull request May 22, 2024
DOCS: fix error in exec namespace note #119378
Merged
@ncoghlan

ncoghlan commented May 22, 2024

Copy link
Copy Markdown
Contributor

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

gpshead reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ncoghlan added a commit that referenced this pull request May 22, 2024
@ncoghlan
DOCS: fix error in exec namespace note (gh-119378)
31d61a7
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.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 22, 2024
@ncoghlan @miss-islington
DOCS: fix error in exec namespace note (pythongh-119378)
fc3310e
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 <[email protected]>
@miss-islington miss-islington mentioned this pull request May 22, 2024
[3.13] DOCS: fix error in exec namespace note (gh-119378) #119380
Merged
ncoghlan added a commit that referenced this pull request May 22, 2024
@miss-islington @ncoghlan
[3.13] DOCS: fix error in exec namespace note (gh-119380)
b5b0e32
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 <[email protected]>
@hoodmane

hoodmane commented May 22, 2024

Copy link
Copy Markdown
Contributor Author

I think what people may want when they pass locals is for globals to work like a custom overlay over builtins and locals to work like globals? I wonder if that is possible.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gpshead

gpshead commented May 22, 2024

Copy link
Copy Markdown
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
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JelleZijlstra JelleZijlstra mentioned this pull request May 28, 2024
backport a9a74da 3.13 #119642
Closed
@CNSeniorious000

CNSeniorious000 commented Jun 3, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

for globals to work like a custom overlay over builtins and locals to work like globals? I wonder if that is possible.

I think the best practise to implement layered context is to use ChainMap.

Let's say you have 2 maps, a Mapping global_namespace (acts as the readonly fallback layer, like __builtins__) and a MutableMapping local_namespace (acts as the globals). And build a map namespace = ChainMap(local_namespace, global_namespace) and getitem will first try local_namespace than try global_namespace, while setitem will never modify local_namespace.

But! eval's globals can only be a dict, not a ChainMap, so as to pyodide.runPython. So you have to use a mixin hack like this:

from collections import ChainMap

class ChainMap(ChainMap, dict):
    pass

Reproduction:

>>> from collections import ChainMap

>>> g, l = {"a": 2}, {}

>>> source = """
... def f():
...     return g() + 1
...
... def g():
...     return a
...
... a = f()
... """

>>> exec(source, ChainMap(l, g))
Traceback (most recent call last):
  File "<console>", line 1, in <module>
TypeError: globals must be a real dict; try eval(expr, {}, mapping)

>>> class ChainMap(ChainMap, dict):
...     pass
...
>>> exec(source, ChainMap(l, g))

>>> g
{'a': 2}

>>> l
{'f': <function f at 0xa04368>, 'g': <function g at 0x12654d0>, 'a': 3}

I think its a common use case to avoid pollution on the global namespace when using exec and eval, so maybe CPython can make ChainMap to inherit from dict so that we don't need to mixin anymore?

hoodmane reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

This was referenced Jul 16, 2024
Eval/exec and comprehension scopes unclear in documentation #87771
Closed
gh-87771: Improve documentation of eval() #100003
Closed
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@hoodmane @estyxx
DOCS: Suggest always calling exec with a globals argument and no loca…
361a106
…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()
    """)
```
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@ncoghlan @estyxx
DOCS: fix error in exec namespace note (pythongh-119378)
b84a347
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@gpshead gpshead

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@hoodmane @ncoghlan @gpshead @CNSeniorious000
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.