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
lysnikolaou merged 104 commits into from
Apr 30, 2025
Merged

gh-132661: Implement PEP 750 #132662

lysnikolaou merged 104 commits into from
Apr 30, 2025

Conversation

@lysnikolaou
Copy link
Member

@lysnikolaoulysnikolaou 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/

lysnikolaouand 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>
@sobolevn
Copy link
Member

sobolevn commented Apr 28, 2025

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

@lysnikolaou
Copy link
MemberAuthor

lysnikolaou commented Apr 28, 2025

Oops, I was too late. 😆

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

@picnixzpicnixz 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

@markshannonmarkshannon 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-appbot 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
MemberAuthor

lysnikolaou commented Apr 29, 2025

@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

sobolevn commented Apr 29, 2025

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

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

bedevere-bot commented Apr 29, 2025

🤖 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-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2025
@markshannon
Copy link
Member

markshannon commented Apr 29, 2025

@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.

@markshannonmarkshannon self-requested a review April 29, 2025 17:18
dylwil3
dylwil3 reviewed Apr 29, 2025
View reviewed changes
@lysnikolaoulysnikolaou merged commit 6020260 into python:mainApr 30, 2025
72 checks passed
@dvarrazzodvarrazzo mentioned this pull request May 1, 2025
Closed
@tconley1428tconley1428 mentioned this pull request Oct 16, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra left review comments

@StanFromIrelandStanFromIrelandStanFromIreland left review comments

@picnixzpicnixzpicnixz approved these changes

@markshannonmarkshannonmarkshannon approved these changes

@AA-TurnerAA-TurnerAA-Turner approved these changes

@pablogsalpablogsalpablogsal approved these changes

@brandtbucherbrandtbucherbrandtbucher approved these changes

@ZeroIntensityZeroIntensityZeroIntensity approved these changes

@sobolevnsobolevnsobolevn approved these changes

@Eclips4Eclips4Eclips4 approved these changes

@hugovkhugovkhugovk approved these changes

@isidenticalisidenticalAwaiting requested review from isidenticalisidentical is a code owner

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@iritkatrieliritkatrielAwaiting requested review from iritkatrieliritkatriel is a code owner

@savannahostrowskisavannahostrowskiAwaiting requested review from savannahostrowskisavannahostrowski is a code owner

@erlend-aaslanderlend-aaslandAwaiting requested review from erlend-aaslanderlend-aasland is a code owner

@corona10corona10Awaiting requested review from corona10

@AlexWaygoodAlexWaygoodAwaiting requested review from AlexWaygoodAlexWaygood is a code owner

@ezio-melottiezio-melottiAwaiting requested review from ezio-melottiezio-melotti is a code owner

+2 more reviewers

@dylwil3dylwil3dylwil3 left review comments

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

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

20 participants

@lysnikolaou@Wingysam@hugovk@bedevere-bot@sobolevn@AA-Turner@ZeroIntensity@markshannon@JelleZijlstra@vasily-v-ryabov@picnixz@pablogsal@brandtbucher@dylwil3@Eclips4@StanFromIreland@koxudaxi@davepeck@terryjreedy@pauleveritt