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 — gh-74929: Rudimentary docs for PEP 667 by gvanrossum · Pull Request #118581 · python/cpython

Conversation

@gvanrossum

@gvanrossum gvanrossum commented May 4, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

This is not sufficient for the final 3.13 release, but it may have to do for beta 1:

  • What's new entry
  • Updated changelog entry (news blurb)
  • Mention the proxy for f_globals in the datamodel and Python frame object docs

This doesn't have any C API details (what's new refers to the PEP).

For some reason I cannot get the "versionchanged" note in the datamodel docs to show. @hugovk ?

  • Issue: Local variable assignment is broken when combined with threads + tracing + closures #74929

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

Sorry, something went wrong.

Uh oh!

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

All reactions
gvanrossum added 2 commits May 4, 2024 13:16
@gvanrossum
Improve wording of PEP 667 news blurm
1871ac1
@gvanrossum
Add rudimentary docs for PEP 667
192596c
@gvanrossum
gvanrossum requested review from AlexWaygood, gaogaotiantian and markshannon May 4, 2024 20:39
@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir awaiting core review labels May 4, 2024
@bedevere-app bedevere-app Bot mentioned this pull request May 4, 2024
Local variable assignment is broken when combined with threads + tracing + closures #74929
Closed
AlexWaygood
AlexWaygood reviewed May 4, 2024
View reviewed changes
Comment thread Doc/reference/datamodel.rst Resolved

Uh oh!

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

@gvanrossum
Fix versionchanged:: markup
0182e4b
@gvanrossum
gvanrossum requested a review from AlexWaygood May 4, 2024 20:50
@gvanrossum
Improve phrasing in datamodel
57e1d54
@gaogaotiantian

gaogaotiantian commented May 4, 2024

Copy link
Copy Markdown
Member

There's a minor technicality here that might worth clarification.

import sys
frame1 = [sys._getframe() for a in [0]][0]
frame2 = sys._getframe()
assert frame1 is frame2

local1 = [sys._getframe().f_locals for a in [0]][0]
local2 = sys._getframe().f_locals
assert local1 != local2

Whether f_locals is a proxy or a dict technically does not entirely depend on the frame. It depends on when is the frame.f_locals call made - inside a comprehension or not.

As you can tell from the example above, the frame is exactly the same frame object, but the f_locals is different.

efimov-mikhail 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.

@gvanrossum

gvanrossum commented May 4, 2024

Copy link
Copy Markdown
Member Author

There's a minor technicality here that might worth clarification. [...]

That's very subtle. Is it explained in PEP 667? I'm not sure how to work this in the existing (pretty sparse) docs for f_locals.

All reactions

Sorry, something went wrong.

Uh oh!

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

@gaogaotiantian

gaogaotiantian commented May 4, 2024

Copy link
Copy Markdown
Member

Actually the final behavior we agreed on is slightly different from what PEP 667 proposed. PEP 667 also did not mention how f_locals should work with inline comprehensions. Can we modify PEP 667 after it was accepted? I don't know if we should dump all the details in the f_locals docs because it would be rare for the users to use f_locals inside the comprehension so no need to confuse them, but it would be nice to explain it well in PEP 667.

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum

gvanrossum commented May 4, 2024

Copy link
Copy Markdown
Member Author

Actually the final behavior we agreed on is slightly different from what PEP 667 proposed. PEP 667 also did not mention how f_locals should work with inline comprehensions. Can we modify PEP 667 after it was accepted? I don't know if we should dump all the details in the f_locals docs because it would be rare for the users to use f_locals inside the comprehension so no need to confuse them, but it would be nice to explain it well in PEP 667.

Assuming this is about clarifying something that the original text of the PEP did not specify precisely enough, and given that the PEP is brand new, I think it's okay to clarify in the PEP. Although in general PEPs are nowadays seen as historic documents, not as living specifications.

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.

@AlexWaygood

AlexWaygood commented May 4, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

It's not something that has to be done in this PR, but if we think that there are some subtleties here that warrant detailed explanation and that the datamodel docs aren't the right place to do that, one option would be to add a howto similar to the one Larry added in 3.10 to explain the semantics of the __annotations__ attribute: https://docs.python.org/3/howto/annotations.html

It wouldn't really be a howto in the Diataxis sense of the term, but we don't really have a better place for that kind of in-depth article in the CPython docs currently, and there are lots of other articles in that directory that also don't meet Diataxis's strict definition of a "howto" article. And as @gvanrossum says, all the relevant information regarding the semantics here should ideally be findable in the CPython docs somewhere, without having to refer to the PEP, which is a historical document now that it has been accepted.

(But we can probably update the PEP as well, given that it's only just been accepted :-)

All reactions

Sorry, something went wrong.

Uh oh!

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

@gvanrossum

gvanrossum commented May 4, 2024

Copy link
Copy Markdown
Member Author

@AlexWaygood @gaogaotiantian Are you two okay with me merging this? I'm all for iterating later, but I feel it's important to mention at least this much in the beta 1 docs, which I expect will reach a relatively big audience of people who are thinking of testing the beta.

All reactions

Sorry, something went wrong.

Uh oh!

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

gaogaotiantian
gaogaotiantian approved these changes May 4, 2024
View reviewed changes
@gaogaotiantian

gaogaotiantian commented May 4, 2024

Copy link
Copy Markdown
Member

Yes this feels okay for beta.

All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood
AlexWaygood approved these changes May 5, 2024
View reviewed changes
Comment thread Doc/whatsnew/3.13.rst

* :pep:`667`: :attr:`FrameType.f_locals <frame.f_locals>` when used in
a function now returns a write-through proxy to the frame's locals,
rather than a ``dict``. See the PEP for corresponding C API changes

@AlexWaygood AlexWaygood May 5, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Suggested change
rather than a ``dict``. See the PEP for corresponding C API changes
rather than a :class:`dict`. See the PEP for corresponding C API changes

Sorry, something went wrong.

Uh oh!

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

All reactions

@gvanrossum gvanrossum May 5, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think that would be a case of over-linking. Surely readers who know what f_locals is will be familiar with dict.

Sorry, something went wrong.

Uh oh!

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

All reactions

@AlexWaygood AlexWaygood May 5, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sure, I don't feel strongly, feel free to ignore

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Misc/NEWS.d/next/Core and Builtins/2024-04-27-21-44-40.gh-issue-74929.C2nESp.rst Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels May 5, 2024
AlexWaygood
AlexWaygood reviewed May 5, 2024
View reviewed changes
Comment thread Doc/reference/datamodel.rst
Comment on lines +1345 to +1349
If the frame refers to a function or comprehension,
this may return a write-through proxy object.

.. versionchanged:: 3.13
Return a proxy for functions and comprehensions.

@AlexWaygood AlexWaygood May 5, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

And apparently this also applies for type parameter scopes? Though I have no idea if that was deliberate:

>>> import sys
>>> type T[X: type(sys._getframe().f_locals)] = ...
>>> T.__type_params__[0].__bound__
<class 'FrameLocalsProxy'>

Sorry, something went wrong.

Uh oh!

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

All reactions
@gvanrossum
Fix whitespace
b94c068
@gvanrossum
gvanrossum enabled auto-merge (squash) May 5, 2024 15:16
@gvanrossum
gvanrossum merged commit 9c13d9e into python:main May 5, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label May 5, 2024
@gvanrossum
gvanrossum deleted the whatsnew-f-locals branch May 5, 2024 18:47
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@gvanrossum @SonicField
pythongh-74929: Rudimentary docs for PEP 667 (python#118581)
690ffaa
This is *not* sufficient for the final 3.13 release, but it will do for beta 1:

- What's new entry
- Updated changelog entry (news blurb)
- Mention the proxy for f_globals in the datamodel and Python frame object docs

This doesn't have any C API details (what's new refers to the PEP).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes
@gaogaotiantian gaogaotiantian gaogaotiantian approved these changes
@markshannon markshannon Awaiting requested review from markshannon

Uh oh!

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

Assignees

No one assigned

Labels

docs Documentation in the Doc dir

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.

3 participants

@gvanrossum @gaogaotiantian @AlexWaygood
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.