Skip to content

gh-132661: Implement PEP 750 #132662

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 104 commits into from
Apr 30, 2025
Merged

gh-132661: Implement PEP 750 #132662

merged 104 commits into from
Apr 30, 2025

Conversation

lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented Apr 17, 2025 •
edited by github-actions bot
Loading

PEP 750 implementation! 🎉

Let's make sure as many people as possible take a look at this, since it's touching many areas that I've not worked on before.

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

lysnikolaou and others added 30 commits October 18, 2024 17:41
@lysnikolaou
Implement next version of PEP 750
9248287 
Still buggy
@lysnikolaou
First fixes; conv and format_spec still not working
450d771
@lysnikolaou
Change BUILD_INTERPOLATION opcode to simplify
a095a9b
@lysnikolaou
Add templatelib module & clean-up types/objects etc.
499d70c
@lysnikolaou
Support explicit concatenation of template and str
f742d45
@lysnikolaou
Avoid folding one node template into its content
0dda3f9
@lysnikolaou
Implement __eq__ for template and interpolation
17014e7
@lysnikolaou
Fix debug specifier segfault
94371bd
@lysnikolaou
Add __match_args__ to Interpolation
99184cc
@lysnikolaou
Fix syntax error prefix in t-strings
4e89c87
@lysnikolaou
Fix test_dis
6bfb095
@lysnikolaou
Fix test_fstring
560e957
@lysnikolaou
Fix test_embed
4d172c2
@lysnikolaou
Implement interleaving, hash functions and *args __init__
c9362e3
@lysnikolaou
Use Argument Clinic for interpolation_new
d2bd673
@lysnikolaou
Revert to using different grammar rules for tstring due to format_spec
7ae1b60
@lysnikolaou
Fix code generation for when there's many values in tstring
5e7304f
@lysnikolaou
Remove duplicate parens from template repr
c8f22f0
@lysnikolaou
Fix conversion converter for interpolation __new__
b0dbe27
@lysnikolaou
Merge branch 'main' into tstrings
b07c419
@lysnikolaou
Fix test_embed and test_dis -- again
3e84771
@lysnikolaou
Implement implicit concatenation with TemplateStr
9380ab9
@lysnikolaou
Merge branch 'main' into tstrings
22f9dc6
@lysnikolaou
Implement new specification of template type
f207637
@lysnikolaou
Replace nb_add with sq_concat
29b1386
@lysnikolaou
Correctly implement str + tstring
aad5d67
@Wingysam
Make changes requested by Steering Council (#57)
77822be 
* Rename expr to expression, conv to conversion
* Move templatelib under string lib
* Add strings lib to LIBSUBDIRS
* Update type of Template/TemplateIter/Interpolation
@lysnikolaou
Merge branch 'main' into tstrings
f3ba0fe
@koxudaxi @lysnikolaou
Remove unnecessary _Py_TPFLAGS_MATCH_SELF from Template type (#60)
422a515
@koxudaxi @lysnikolaou
Fix memory leak in template concat error handling (#59)
bb7359a 
Co-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
@lysnikolaou
Copy link
Member Author

Should we run buildbots one more time? They're weren't green when they were ran originally.

Yes, I'll run them again before merging, after I've pulled main in. They weren't all green originally, but I've rerun them since then and they were green the second time (only one exception with an unrelated failure).

@sobolevn
Copy link
Member

Oh, I messed up, sorry. I clicked "merge main", but it was a long process :(

@lysnikolaou
Copy link
Member Author

Oops, I was too late. 😆

picnixz
picnixz approved these changes Apr 28, 2025
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

One question and LGTM.

markshannon
markshannon requested changes Apr 29, 2025
View reviewed changes
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

I'm not convinced about adding string/templatelib.py instead of adding the new types to types.
Either way, the C module is unnecessary.

@bedevere-app
Copy link

bedevere-app bot commented Apr 29, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@lysnikolaou
Copy link
Member Author

@markshannon Thanks for the review! I've removed the _templatelib extension.

Regarding the types vs string.templatelib discussion, I don't have a strong opinion either way, but it feels that it's too late for it to be reopened. The SC made their decision and I think we should go with it. We're a week away from the beta freeze and I'd like to get this PR in.

@sobolevn
Copy link
Member

In my humble opinion we should not add to types what we can add to some other semantically named module.

@lysnikolaou lysnikolaou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @lysnikolaou for commit 0b1aef9 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F132662%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2025
@markshannon
Copy link
Member

@markshannon Thanks for the review! I've removed the _templatelib extension.

Regarding the types vs string.templatelib discussion, I don't have a strong opinion either way, but it feels that it's too late for it to be reopened. The SC made their decision and I think we should go with it. We're a week away from the beta freeze and I'd like to get this PR in.

I'm fine with you merging this now. It is only a small change to move the code to types.py in another PR if that's what we decide we want.

@markshannon markshannon self-requested a review April 29, 2025 17:18
dylwil3
dylwil3 reviewed Apr 29, 2025
View reviewed changes
@lysnikolaou lysnikolaou merged commit 6020260 into python:main Apr 30, 2025
72 checks passed
@dvarrazzo dvarrazzo mentioned this pull request May 1, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@StanFromIreland StanFromIreland StanFromIreland left review comments

@dylwil3 dylwil3 dylwil3 left review comments

@vasily-v-ryabov vasily-v-ryabov vasily-v-ryabov left review comments

@picnixz picnixz picnixz approved these changes

@markshannon markshannon markshannon approved these changes

@AA-Turner AA-Turner AA-Turner approved these changes

@pablogsal pablogsal pablogsal approved these changes

@brandtbucher brandtbucher brandtbucher approved these changes

@ZeroIntensity ZeroIntensity ZeroIntensity approved these changes

@sobolevn sobolevn sobolevn approved these changes

@Eclips4 Eclips4 Eclips4 approved these changes

@hugovk hugovk hugovk approved these changes

@isidentical isidentical Awaiting requested review from isidentical isidentical is a code owner

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner

@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski savannahostrowski is a code owner

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

@corona10 corona10 Awaiting requested review from corona10

@AlexWaygood AlexWaygood Awaiting requested review from AlexWaygood AlexWaygood is a code owner

@ezio-melotti ezio-melotti Awaiting requested review from ezio-melotti ezio-melotti is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.