Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.7k
PEP 667: Clarify specification ambiguities and the expected impact#3845
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
gaogaotiantian commented Jun 19, 2024 • edited by ncoghlan
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by ncoghlan
Uh oh!
There was an error while loading. Please reload this page.
carljm left a comment
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.
This looks really good!
I think it might also make sense to add a "Rejected Alternatives" section, with a sub-section "Maintaining existing behavior of exec", discussing some of what we talked about in Discourse about the problems with trying to do this.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ncoghlan commented Jun 19, 2024
The new section looks good to me. FWIW, What's New in 3.13 does discuss this aspect of the change (https://docs.python.org/dev/whatsnew/3.13.html#defined-mutation-semantics-for-locals), but having more details in the PEP definitely won't hurt. |
ncoghlan commented Jun 19, 2024
It may also be worth referencing back to the PEP 558 discussions of the impact of changing the way
And while PEP 667's text didn't explicitly mention the impact on This is a case where the use of competing PEPs may have let us down a bit - PEP 667 (quite reasonably) didn't bother repeating the rationale for the aspects where it agreed with the design decisions in PEP 558, so reading it in isolation from PEP 558 doesn't quite give the full picture of the change. |
markshannon left a comment
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.
gaogaotiantian commented Jun 20, 2024
So should I post this on disclosure first? |
Uh oh!
There was an error while loading. Please reload this page.
carljm commented Jun 20, 2024
Yes, this should be posted on Discourse to get community feedback, but it probably makes sense to address the existing feedback first? |
Co-authored-by: Carl Meyer <carl@oddbird.net>
gaogaotiantian commented Jun 21, 2024
Of course, just want to make sure about the procedure. I'm a bit swamped recently. |
ncoghlan commented Jun 21, 2024
I ticked the SC approval box in the template, since we have that in python/steering-council#245 (comment) |
carljm commented Jun 21, 2024
I could be misunderstanding, but I don't see an indication in that comment that the SC considered the issue of |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Carl Meyer <carl@oddbird.net>
willingc commented Jun 21, 2024
@gaogaotiantian I accepted @carljm's suggestions so that I could look at the prose as it would display to a reader. I am going to add a few suggestions re: grammar, and then I believe this will be acceptable from my viewpoint. Thanks so much for doing this. It adds important clarity for all. 💐 |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
markshannon left a comment
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.
Apologies for not getting to this sooner.
A few suggestions for moving stuff around a bit and a couple of very minor corrections.
Otherwise this looks good.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Mark Shannon <mark@hotpy.org>
ncoghlan left a comment • 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.
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.
Mark's comments all make sense to me, so I'm working on an update now to address the ones which aren't simple line edits (I already accepted the simple changes).
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
* Move "Implementation Notes" after implementation section * Avoid jargon when describing the deprecation notice plan * Add inline comments to PyEval_GetLocals() replacement code Additional changes: * Add 2024 post-history entry * Move PEP 709 impact to Backwards Compatibility section
ncoghlan left a comment
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.
Updates pushed to address most of @markshannon's feedback, but after some experimentation at the REPL, I still need to make some more changes to the paragraph about deleting values via frame proxy instances.
Uh oh!
There was an error while loading. Please reload this page.
markshannon left a comment
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.
This looks really good now. Thanks.
ncoghlan commented Oct 21, 2024
It was a journey, but we got there in the end :) Thank you all. I have to share this bit from the generated squash merge showing how much of a group effort it really was: (and of course @gaogaotiantian as the original PR author) |
gaogaotiantian commented Oct 21, 2024
Thank you @ncoghlan for taking this over. I wouldn't be able to finish it like this :) |
Vako111 left a comment
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.
Good
Vako111 left a comment
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.
He
…ython#3845) --------- Co-authored-by: Carl Meyer <carl@oddbird.net> Co-authored-by: Carol Willing <carolcode@willingconsulting.com> Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com> Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: Mark Shannon <mark@hotpy.org>
…ython#3845) --------- Co-authored-by: Carl Meyer <carl@oddbird.net> Co-authored-by: Carol Willing <carolcode@willingconsulting.com> Co-authored-by: Alyssa Coghlan <ncoghlan@gmail.com> Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: Mark Shannon <mark@hotpy.org>
PEP 123: Summary of changes)📚 Documentation preview 📚: https://pep-previews--3845.org.readthedocs.build/