You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ast.dump() elision of empty values should use field types #134718
In 3.13 we changed ast.dump() to not show empty values (None or the empty list) by default. However, this is based purely on the value of individual attributes:
Instead, this logic should look at the node's _field_types and use the same logic we use to determine whether to allow omitting the argument in calls to the constructor: elide None if the type is a union including None, elide [] if the type is a list, and elide Load() if the type is an expr_context.
I've been working on the missing None when show_empty=True. I tried to integrate into the current function.
ast.dump
from_astimportAST, Constant, MatchSingleton, MatchStardefdump(
node, annotate_fields=True, include_attributes=False,
*,
indent=None, show_empty=False,
):
""" Return a formatted dump of the tree in node. This is mainly useful for debugging purposes. If annotate_fields is true (by default), the returned string will show the names and the values for fields. If annotate_fields is false, the result string will be more compact by omitting unambiguous field names. Attributes such as line numbers and column offsets are not dumped by default. If this is wanted, include_attributes can be set to true. If indent is a non-negative integer or string, then the tree will be pretty-printed with that indent level. None (the default) selects the single line representation. If show_empty is False, then empty lists and fields that are None will be omitted from the output for better readability. """def_format(node, level=0):
ifindent_strisnotNone:
level+=1prefix='\n'+indent_str*levelsep=',\n'+indent_str*levelelse:
prefix=''sep=', 'ifisinstance(node, AST):
cls=type(node)
args= []
args_buffer= []
allsimple=Truekeywords=annotate_fieldsfornameinnode._fields:
try:
value=getattr(node, name)
exceptAttributeError:
keywords=TruecontinueifvalueisNoneandgetattr(cls, name, ...) isNone:
ifshow_empty:
args.append('%s=%s'% (name, value))
keywords=Truecontinueelif (
notshow_emptyand (valueisNoneorvalue== [])
# Special cases:# `Constant(value=None)` and `MatchSingleton(value=None)` and `MatchStar(name=None)`andnotisinstance(node, (Constant, MatchSingleton, MatchStar))
):
args_buffer.append(repr(value))
continueelifnotkeywords:
args.extend(args_buffer)
args_buffer= []
value_formatted, simple=_format(value, level)
allsimple=allsimpleandsimpleifkeywords:
args.append('%s=%s'% (name, value_formatted))
else:
args.append(value_formatted)
ifinclude_attributesandnode._attributes:
forname_attributesinnode._attributes:
try:
value_attributes=getattr(node, name_attributes)
exceptAttributeError:
continueifvalue_attributesisNoneandgetattr(cls, name_attributes, ...) isNone:
continuevalue_attributes_formatted, simple=_format(value_attributes, level)
allsimple=allsimpleandsimpleargs.append('%s=%s'% (name_attributes, value_attributes_formatted))
ifallsimpleandlen(args) <=3:
return ('%s(%s)'% (f"{node.__class__.__module__}.{node.__class__.__name__}", ', '.join(args)), notargs)
return ('%s(%s%s)'% (f"{node.__class__.__module__}.{node.__class__.__name__}", prefix, sep.join(args)), False)
elifisinstance(node, list):
ifnotnode:
return ('[]', True)
return'[%s%s]'% (prefix, sep.join(_format(x, level)[0] forxinnode)), Falsereturn (repr(node), True)
ifnotisinstance(node, AST):
raiseTypeError('expected AST, got %r'%node.__class__.__name__)
ifindentisnotNoneandnotisinstance(indent, str):
indent_str=' '*indentelse:
indent_str=indentreturn_format(node)[0]
FWIW, I think the elided idea for show_empty=False is far better than
elif (
notshow_emptyand (valueisNoneorvalue== [])
# Special cases:# `Constant(value=None)` and `MatchSingleton(value=None)`andnotisinstance(node, (Constant, MatchSingleton))
):
I didn't like that code block, but I didn't know enough to figure out a different way.
I have a request for whomever rewrites the function: please change the identifiers so that a stub file can type the identifiers. There is a difference between "not typed" and "not typeable."
This is a great idea. There is an issue with the current code -- it omits None and [] not only for optional arguments. It has a special case for Constant and MatchSingleton which can legally have the non-optional None value, but a new Interpolation node was added recently. It's str parameter has type constant, so it can theoretically be any value.
Also, None and [] wrongly assigned to other attributes should not be omitted.
For None, we can detect an optional argument by simply testing if the class variable was set to None. For [], the only way to do this is using a field_type.
So this part is qualified as a bug fix and should be backported back to 3.13. Omission of the Load() value is a new feature.
Sorry, @grantlouisherman, there were some nontrivial issues here to left the matter for new contributors. Ultimately, this resulted in three PRs that were backported independently.
Activity
hunterhogan commentedon May 26, 2025
I've been working on the missing
Nonewhenshow_empty=True. I tried to integrate into the current function.ast.dump
type annotations
stdlib/builtins.pyi type change
side by side comparison file
I guess add
dump, above, to the file below instead of the import I'm using.hunterhogan commentedon May 26, 2025
FWIW, I think the elided idea for
show_empty=Falseis far better thanI didn't like that code block, but I didn't know enough to figure out a different way.
I have a request for whomever rewrites the function: please change the identifiers so that a stub file can type the identifiers. There is a difference between "not typed" and "not typeable."
grantlouisherman commentedon May 28, 2025
@hunterhogan could I take a look at this? Im new but wanted to take a crack if thats ok.
hunterhogan commentedon May 29, 2025
@grantlouisherman Sure. I threw everything into a directory here.
https://github.com/hunterhogan/cpython/tree/804191081b49aa1943f8dfcb8055d875ea6f8d07/Z0Z_workingFiles
I have a lightly refactored and functioning version in astToolkit. import astToolkit; astToolkit.dump(...).
The major barrier to the pull request, afaik, is that the current tests are reporting failures because the tests are wrong.
I don't have enough experience with this labyrinthian process or the health to cope and to figure out how to publish working code.
grantlouisherman commentedon May 29, 2025
@serhiy-storchaka are you taking a look? I see you assigned it to yourself. If so Ill look for a new issue.
serhiy-storchaka commentedon May 30, 2025
This is a great idea. There is an issue with the current code -- it omits
Noneand[]not only for optional arguments. It has a special case forConstantandMatchSingletonwhich can legally have the non-optionalNonevalue, but a newInterpolationnode was added recently. It'sstrparameter has typeconstant, so it can theoretically be any value.Also,
Noneand[]wrongly assigned to other attributes should not be omitted.For
None, we can detect an optional argument by simply testing if the class variable was set toNone. For[], the only way to do this is using a field_type.So this part is qualified as a bug fix and should be backported back to 3.13. Omission of the
Load()value is a new feature.pythongh-134718: Fix ast.dump() for empty non-default values
grantlouisherman commentedon May 30, 2025
Hey @serhiy-storchaka it seems you took this Ill find another issue
gh-134718: Fix ast.dump() for empty non-default values (GH-134926)
pythongh-134718: Fix ast.dump() for empty non-default values (pythonG…
pythongh-134718: Omit optional Load() values in ast.dump()
[3.13] pythongh-134718: Fix ast.dump() for empty non-default values (p…
[3.14] gh-134718: Fix ast.dump() for empty non-default values (GH-134926
[3.13] gh-134718: Fix ast.dump() for empty non-default values (GH-134926
gh-134718: Omit optional Load() values in ast.dump() (GH-134934)
serhiy-storchaka commentedon May 31, 2025
Sorry, @grantlouisherman, there were some nontrivial issues here to left the matter for new contributors. Ultimately, this resulted in three PRs that were backported independently.
pythongh-134718: Fix ast.dump() for empty non-default values (pythonG…
pythongh-134718: Omit optional Load() values in ast.dump() (pythonGH-…
pythongh-134718: Fix ast.dump() for empty non-default values (pythonG…
pythongh-134718: Omit optional Load() values in ast.dump() (pythonGH-…
pythongh-134718: Fix ast.dump() for empty non-default values (pythonG…