FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-116021: Deprecate support for instantiating abstract AST nodes by brianschubert · Pull Request #137865 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-116021: Deprecate support for instantiating abstract AST nodes - #137865

Merged
JelleZijlstra merged 16 commits into
python:mainfrom
brianschubert:gh-116021-ast-depr-abstract
May 2, 2026
ConversationCommits (16)ChecksFiles changed
Merged

gh-116021: Deprecate support for instantiating abstract AST nodes#137865
JelleZijlstra merged 16 commits into
python:mainfrom
brianschubert:gh-116021-ast-depr-abstract

Conversation

brianschubert commented Aug 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Demonstration:

>>> ast.stmt()
<python-input-1>:1: DeprecationWarning: Instantiating abstract AST node class ast.stmt is deprecated. This will become an error in Python 3.20
stmt()

>>> class MyStmt(ast.stmt): pass
... 
>>> MyStmt()
MyStmt()

This tracks which AST nodes are abstract using an internal set that gets built during node type initialization. I tried a few different approaches, and this seemed the simplest. A nice thing about this approach is that the check for abstract-ness depends on the exact node type, so this avoids most inheritance subtleties that could interfere with existing user-defined subclasses.

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

bedevere-app Bot mentioned this pull request Aug 16, 2025
Closed
brianschubert commented Aug 16, 2025
View reviewed changes
Comment thread Lib/test/test_sys.py Outdated
Comment on lines +1880 to +1881
check(_ast.AST(), size('P'))
with self.assertWarns(DeprecationWarning):
check(_ast.AST(), size('P'))

brianschubert Aug 16, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this should probably be rewritten, but I'm not sure I understand the goal of this test well enough to do that. Suggestions welcome!

picnixz Aug 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's to check that objects in Python/ have the correct types. I don't know why it's actually in test_sys though. I think we should split this test and move each check to the module it belongs (ast.AST's size should be checked in test_ast).

For now, leave it here. A separate issue should be opened.

JelleZijlstra Oct 20, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think it's here because it's arguably testing sys.getsizeof. Maybe it could be changed to test a concrete AST class instead?

brianschubert Oct 27, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

check(_ast.Module(), size('3P')) might work then? (not sure if I'm counting the object size right)

brianschubert Nov 29, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@JelleZijlstra @picnixz any further thoughts re: above? Happy to update this is someone can confirm a good substitute

JelleZijlstra Dec 2, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, what you suggest should work (haven't verified the object size is right).

brianschubert Feb 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looking again 3P seems right, updated!

picnixz reviewed Aug 18, 2025
View reviewed changes

picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would it be easier to have a module function checking if a node is abstract? say _ast.is_abstract(node) (in the C module, not the Python module). That way we don't need an additional method on AST objects (which could be shadowed).

Comment thread Lib/test/test_sys.py Outdated
Comment on lines +1880 to +1881
check(_ast.AST(), size('P'))
with self.assertWarns(DeprecationWarning):
check(_ast.AST(), size('P'))

picnixz Aug 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's to check that objects in Python/ have the correct types. I don't know why it's actually in test_sys though. I think we should split this test and move each check to the module it belongs (ast.AST's size should be checked in test_ast).

For now, leave it here. A separate issue should be opened.

Comment thread Parser/asdl_c.py
Comment thread Parser/asdl_c.py Outdated
return result;
}

/* Helper for checking if a node class is abstract in the tests. */

picnixz Aug 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think we should use clinic here, but this would create a new file as we don't use it at all. Maybe we can do it in a follow-up PR though.

Comment thread Parser/asdl_c.py Outdated
Comment thread Lib/test/test_ast/test_ast.py Outdated
Comment thread Lib/test/test_ast/test_ast.py Outdated
Comment thread Lib/test/test_ast/test_ast.py Outdated
Comment thread Lib/test/test_ast/test_ast.py Outdated
Comment thread Lib/test/test_ast/test_ast.py
JelleZijlstra approved these changes Oct 20, 2025
View reviewed changes
Comment thread Lib/test/test_sys.py Outdated
Comment on lines +1880 to +1881
check(_ast.AST(), size('P'))
with self.assertWarns(DeprecationWarning):
check(_ast.AST(), size('P'))

JelleZijlstra Oct 20, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think it's here because it's arguably testing sys.getsizeof. Maybe it could be changed to test a concrete AST class instead?

JelleZijlstra reviewed Feb 22, 2026
View reviewed changes
Comment thread Parser/asdl_c.py Outdated
brianschubert and others added 2 commits February 22, 2026 17:20
Update Parser/asdl_c.py
b7100a4 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Regen Python-ast.c
202a372

github-actions Bot commented Apr 28, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 28, 2026
JelleZijlstra merged commit bdedc4a into python:main May 2, 2026
98 of 100 checks passed
newville mentioned this pull request Oct 2, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

JelleZijlstra JelleZijlstra approved these changes
Eclips4 Eclips4 approved these changes
AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
tomasr8 Awaiting requested review from tomasr8 tomasr8 is a code owner
isidentical Awaiting requested review from isidentical
ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
pablogsal Awaiting requested review from pablogsal pablogsal is a code owner
lysnikolaou Awaiting requested review from lysnikolaou lysnikolaou is a code owner
picnixz Awaiting requested review from picnixz

Assignees

No one assigned

Labels

stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL