Skip to content

gh-133555: Allow to regenerate the parser with Python < 3.14 #133557

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 5 commits into from
May 8, 2025
Merged

gh-133555: Allow to regenerate the parser with Python < 3.14 #133557

merged 5 commits into from
May 8, 2025

Conversation

alexprengere
Copy link
Contributor

@alexprengere alexprengere commented May 7, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request May 7, 2025
Closed
lysnikolaou
lysnikolaou approved these changes May 7, 2025
View reviewed changes
Copy link
Member

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM! Thanks @alexprengere!

@AA-Turner
Copy link
Member

Should we add a test for this?

@lysnikolaou
Copy link
Member

Hmm, not sure what the test would look like. If it's verifying the list of token, we would have to update every time a new token is added, which is as easy to miss as just changing the ifs like in this PR.

And I don't think we have the ability to run scripts with different python versions to verify they work, right?

@encukou
Copy link
Member

encukou commented May 7, 2025

Maybe add a reminder comment to e.g. Grammar/Tokens?

@AA-Turner
Copy link
Member

And I don't think we have the ability to run scripts with different python versions to verify they work, right?

We could add something to the Check if generated files are up to date CI check perhaps? Though just having a comment may be simpler.

A

@python-cla-bot
Copy link

python-cla-bot bot commented May 7, 2025 •
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

@alexprengere
Copy link
Contributor Author

After committing @AA-Turner suggestion directly from the GitHub UI, the CLA bot now require signing for the noreply mail used. Should I edit the commit manually to update the mail address, or is there a another way to do this?

@AA-Turner
Copy link
Member

If you click the 'CLA not signed' button, you should be able to sign it. If you have problems, can you paste a screenshot of what you see?

A

@alexprengere
Copy link
Contributor Author

All good now, thanks! I was not sure if we were supposed to sign those noreply mail addresses as well (my primary gmail addresses was already signed).

@lysnikolaou
Copy link
Member

@alexprengere Could you add a comment to Grammar/Tokens that the peg generator (ideally mentioning specific files and lines) needs to be udpated as well when adding new tokens?

@alexprengere
Copy link
Contributor Author

Done.
Is there a fundamental reason why python -m pegen python does not take Grammar/Tokens as input, the same way that python -m pegen c does? This would also solve the issue of updating parser_generator.py when new tokens are added.

AA-Turner
AA-Turner approved these changes May 7, 2025
View reviewed changes
@alexprengere alexprengere force-pushed the fix_pegen_tstrings_tokens branch from 3609894 to e1ebec4 Compare May 7, 2025 17:17
@alexprengere alexprengere force-pushed the fix_pegen_tstrings_tokens branch from e1ebec4 to 68066fd Compare May 7, 2025 20:54
@AA-Turner AA-Turner added the needs backport to 3.14 bugs and security fixes label May 8, 2025
@AA-Turner AA-Turner merged commit b48599b into python:main May 8, 2025
48 checks passed
@miss-islington-app
Copy link

Thanks @alexprengere for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 8, 2025
@alexprengere @AA-Turner @miss-islington
pythongh-133555: Allow regenerating the parser with Python < 3.14 (py…
c51f96f 
…thonGH-133557)

(cherry picked from commit b48599b)

Co-authored-by: Alex Prengère <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 8, 2025

GH-133630 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 8, 2025
AA-Turner added a commit that referenced this pull request May 8, 2025
@miss-islington @alexprengere @AA-Turner
[3.14] gh-133555: Allow regenerating the parser with Python < 3.14 (G…
aadfc2c 
…H-133557) (#133630)

gh-133555: Allow regenerating the parser with Python < 3.14 (GH-133557)
(cherry picked from commit b48599b)

Co-authored-by: Alex Prengère <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@lysnikolaou lysnikolaou lysnikolaou approved these changes

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

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

Successfully merging this pull request may close these issues.
4 participants
@alexprengere @AA-Turner @lysnikolaou @encukou