Skip to content

gh-137600: Promote ast node constructor deprecation warnings to errors #137601

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
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

gh-137600: Promote ast node constructor deprecation warnings to errors #137601

wants to merge 10 commits into from

Conversation

brianschubert
Copy link
Member

@brianschubert brianschubert commented Aug 9, 2025 •
edited by github-actions bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Aug 9, 2025
Open
@brianschubert
Copy link
Member Author

@picnixz As mentioned in #121162 (comment), now that the constructors raise on missing and unknown fields, some of the checks that were added for copy.replace support are no longer needed, so I went ahead and removed them (de5564a). Based on the tests, it looks like things are working correctly, but I'd appreciate if you could take a look to make sure I didn't break anything :-)

@brianschubert brianschubert linked an issue Aug 9, 2025 that may be closed by this pull request
Promote ast node constructor deprecation warnings to errors #137600
Open
JelleZijlstra
JelleZijlstra reviewed Aug 10, 2025
View reviewed changes
@picnixz
Copy link
Member

picnixz commented Aug 10, 2025

This takes me back as the original PR is one of my very first contributions to CPython!

@picnixz picnixz self-requested a review August 10, 2025 06:55
picnixz
picnixz reviewed Aug 10, 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.

I'd suggest using msg = ... + re.escape() for assertRaisesRegex() where 80 chars limit is exceeded.

picnixz
picnixz reviewed Aug 10, 2025
View reviewed changes
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

@picnixz picnixz picnixz left review comments

@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner

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

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

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

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

Successfully merging this pull request may close these issues.

Promote ast node constructor deprecation warnings to errors
4 participants
@brianschubert @picnixz @JelleZijlstra @Eclips4