bpo-36287: Make ast.dump() not output optional fields and attributes. by serhiy-storchaka · Pull Request #18843 · python/cpython · GitHub

Values for optional fields and attributes of AST nodes are now set as class attributes (e.g. Constant.kind is set to None).

https://bugs.python.org/issue36287

Values for optional fields and attributes of AST nodes are now set as class attributes (e.g. Constant.kind is set to None).

Suggestion: I think these attributes should still be printed if include_attributes=True in ast.dump(). For instance, we use this in the new PEG parser to quickly compare two full AST trees. It will still work (because we are only skipping them if they are not the default value) but I think it may make sense to have a "full verbose" mode.

What do you think?

This should be necessary for the doctest as we have

   .. testsetup::
    
        import ast

at the beggining of the document. Maybe I am missing something, though

I am not experienced with doctests. I ran ./python -m doctest Doc/library/ast.rst and it failed. Seems doctest does not understand testsetup. Indeed, it is a Sphinx extension.

Yeah, you need to do "make doctest" in Docs or PATH=./venv/bin:$PATH sphinx-build -b doctest -d build/doctrees -W . build/doctest FILENAME if you want to only test one single file (the first run will be a bit longer while it builds the tree).

Currently the only attributes are "lineno", "col_offset", "end_lineno" and "end_col_offset". They have type int, the latter two are optional. When convert AST nodes from C to Python these attributes are always set to integer value (at worst case it is 0). They are never None. When convert from Python to C None will be converted to 0.

Currently if you create an AST node manually and not set attributes, they will not be output by ast.dump().

>>> print(ast.dump(ast.Constant(), include_attributes=True))
Constant()

If always output attributes if include_attributes=True, you would get

>>> print(ast.dump(ast.Constant(), include_attributes=True))
Constant(end_lineno=None, end_col_offset=None)

Is it what you want to get?

Is it what you want to get?

Sort of. What I am proposing (still don't know if its a good idea) is to have a way of "print everything, no matter if is default or not"). Feel free to ignore this if you don't see much value, I just wanted to know if you think it may make sense to preserve that behaviour under some option

LGTM

I think this is a great improvement! Feel free to ignore the remaining suggestions if you don't think is worth tackling those.

I experimented and I do not like the result. It would break an existing test test_dump_incomplete which is not touched by this PR. It is not important. I think that in practical cases there will be not difference because you will always set instance attributes, and they are always different from default values (because they are ints, and the default is None).

In any case, this is just a first step, and there is a place for improvements. This change fixes the most annoying thing, kind=None in Constant, and the rest is less important.

In Python 3.9+, ast.dump() omits optional fields/attributes from the output if their value is None. Such defaults are defined as class attributes. See https://bugs.python.org/issue36287 And python/cpython#18843 This patch does not change the output on previous Python versions, because the class attributes are missing there. Fixes simonpercivall#56

…with default values. (GH-18843) The default values for optional fields and attributes of AST nodes are now set as class attributes (e.g. Constant.kind is set to None).

Do we need to check this twice (additional getattr(cls, name, ...) is None)? It seems like those is Nones will always be the same

Change and test.

Sorry, although I did not change, now I can see that I overlooked that this is cls and not node

this conversation can be resolved, I think

bpo-36287: Make ast.dump() not output optional fields and attributes.

380ea25

Values for optional fields and attributes of AST nodes are now set as class attributes (e.g. Constant.kind is set to None).

serhiy-storchaka added the type-feature A feature request or enhancement label Mar 8, 2020

the-knights-who-say-ni added the CLA signed label Mar 8, 2020

bedevere-bot added the awaiting core review label Mar 8, 2020

Update examples in docs.

61c2fbd

serhiy-storchaka requested a review from pablogsal March 8, 2020 08:12

serhiy-storchaka added 3 commits March 8, 2020 11:36

Remove commented out code.

049ab20

Add an empty line.

cb613cb

Remove commented out lines.

cd96837

pablogsal reviewed Mar 9, 2020

View reviewed changes

Comment thread Misc/NEWS.d/next/Library/2020-03-08-09-53-55.bpo-36287.mxr5m8.rst Outdated

pablogsal reviewed Mar 9, 2020

View reviewed changes

serhiy-storchaka added 2 commits March 9, 2020 21:02

Merge branch 'master' into ast-optional

0fa4d0e

Apply review suggestions.

3a58208

Refactor.

d17cfd7

pablogsal approved these changes Mar 9, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Mar 9, 2020

Merge branch 'master' into ast-optional

2b456bd

serhiy-storchaka merged commit b7e9525 into python:master Mar 9, 2020

bedevere-bot removed the awaiting merge label Mar 9, 2020

serhiy-storchaka deleted the ast-optional branch March 9, 2020 22:07

hroncok mentioned this pull request Jul 8, 2020

Test issues with Python 3.9 simonpercivall/astunparse#56

Open

hroncok mentioned this pull request Jul 8, 2020

Test failures with Python 3.9 serge-sans-paille/gast#46

Closed

0dminnimda reviewed Oct 22, 2021

View reviewed changes

serhiy-storchaka mentioned this pull request May 11, 2023

Make ast.dump() not output optional default fields #80468

Closed

-                                      allsimple = allsimple and simple
-                                      args.append('%s=%s' % (attr, value))
+                                      continue
+                                  if value is None and getattr(cls, name, ...) is None:

bpo-36287: Make ast.dump() not output optional fields and attributes. by serhiy-storchaka · Pull Request #18843 · python/cpython · GitHub

Repository navigation

bpo-36287: Make ast.dump() not output optional fields and attributes. - #18843

bpo-36287: Make ast.dump() not output optional fields and attributes.#18843serhiy-storchaka merged 9 commits intopython:masterpython/cpython:masterfrom serhiy-storchaka:ast-optionalserhiy-storchaka/cpython:ast-optionalCopy head branch name to clipboard

Conversation

serhiy-storchaka commented Mar 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablogsal commented Mar 9, 2020

Uh oh!

Uh oh!

pablogsal Mar 9, 2020

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Mar 9, 2020

Choose a reason for hiding this comment

Uh oh!

pablogsal Mar 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Mar 9, 2020

Uh oh!

pablogsal commented Mar 9, 2020

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Mar 9, 2020

Uh oh!

0dminnimda Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Oct 22, 2021

Choose a reason for hiding this comment

Uh oh!

0dminnimda Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

0dminnimda Oct 23, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bpo-36287: Make ast.dump() not output optional fields and attributes.#18843
serhiy-storchaka merged 9 commits into
python:masterfrom
serhiy-storchaka:ast-optional

serhiy-storchaka commented Mar 8, 2020 •
edited
Loading

pablogsal Mar 9, 2020 •
edited
Loading