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

[serhiy-storchaka]

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

[pablogsal]

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?

[pablogsal]

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

[serhiy-storchaka]

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.

[pablogsal]

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).

[serhiy-storchaka]

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?

[pablogsal]

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

[pablogsal]

LGTM

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

[serhiy-storchaka]

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.

[0dminnimda]

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

[serhiy-storchaka]

Change and test.

[0dminnimda]

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

[0dminnimda]

this conversation can be resolved, I think