classification
Title: Make ast.dump() not output optional default fields
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.9
process
Status: open Resolution:
Dependencies: 39981 Superseder:
Assigned To: Nosy List: BTaskaya, benjamin.peterson, eamanu, levkivskyi, serhiy.storchaka, yselivanov
Priority: normal Keywords: patch

Created on 2019-03-14 09:48 by serhiy.storchaka, last changed 2020-04-08 14:28 by BTaskaya.

Files
File name Uploaded Description Edit
ast.patch BTaskaya, 2020-03-16 10:00
Pull Requests
URL Status Linked Edit
PR 12328 closed eamanu, 2019-03-14 14:35
PR 18843 merged serhiy.storchaka, 2020-03-08 07:57
Messages (16)
msg337907 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-03-14 09:48
Currently ast.dump() outputs values for optional fields even if they are equal to defaults. This makes the output unnecessary verbose.

For example (kind and type_comment are optional):

>>> ast.dump(ast.parse('x = 1'))
"Module(body=[Assign(targets=[Name(id='x', ctx=Store())], value=Constant(value=1, kind=None), type_comment=None)], type_ignores=[])"
msg337922 - (view) Author: Emmanuel Arias (eamanu) * Date: 2019-03-14 14:36
Hi @serhiy.storchaka,

I send a patch to Github to review. Let me know if is necessary unittest. 

Regards
msg337955 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-03-14 17:27
@eamanu tests are basically always necessary. :)
msg338030 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-03-15 20:33
We can probably also skip `type_ignores` list if it is empty (which will be the case in 99% situations).
msg338033 - (view) Author: Emmanuel Arias (eamanu) * Date: 2019-03-15 20:54
Maybe we can ignore None and [] ?
msg338055 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-03-16 06:25
None can not be ignored in Constant(value=None). [] can not be ignored in Tuple(elts=[]).

There is also a problem with using ast.dump() with annotate_fields=False:

>>> from ast import *
>>> dump(Raise(cause=Name(id='B', ctx=Load())), annotate_fields=False)
"Raise(Name('B', Load()))"
>>> dump(Raise(Name('B', Load())))
"Raise(exc=Name(id='B', ctx=Load()))"

For Raise(cause=X) it outputs a string which is evaluated to Raise(exc=X).
msg356925 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python triager) Date: 2019-11-18 22:41
@eamanu are you still interested in this issue? (bumped to 3.9)
msg356935 - (view) Author: Emmanuel Arias (eamanu) * Date: 2019-11-19 00:10
Hi, 

Sorry sincerely I forgot this issue, if there are not any objection I can continue it.
msg357012 - (view) Author: Ivan Levkivskyi (levkivskyi) * (Python committer) Date: 2019-11-20 00:42
No objections from me assuming you are going forward along the way proposed by Serhiy (i.e. only skip values for certain fields if value is the default, not a blanket skip for all Nones and empty lists).
msg363640 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-08 08:33
PR 18843 solves this issue by setting None as class attributes for optional  fields and attributes (like "kind" or "end_col_offset").

It is not so easy for fields like "type_ignores". They are mutable lists, so this approach cannot be applied to them.

I want to get rid also from "ctx=Load()", but it is more complex change.
msg363641 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python triager) Date: 2020-03-08 08:56
> It is not so easy for fields like "type_ignores". They are mutable lists, so this approach cannot be applied to them.

What about keeping ASDL signatures in the nodes (). If we know the type of a field (can be parsed in runtime) we can infer the default value of a field. For type_ignores, it is a sequence so if it is empty we can just crop that part.
msg363642 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python triager) Date: 2020-03-08 08:56
Related issue: issue 39638
msg363775 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-09 22:07
New changeset b7e9525f9c7ef02a1d2ad8253afdeb733b0951d4 by Serhiy Storchaka in branch 'master':
bpo-36287: Make ast.dump() not output optional fields and attributes with default values. (GH-18843)
https://github.com/python/cpython/commit/b7e9525f9c7ef02a1d2ad8253afdeb733b0951d4
msg364305 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python triager) Date: 2020-03-16 10:00
@serhiy.storchaka, with these ASDL signatures, I have a patch that would omit default values for both Nones and [] in case they are redundant. But this is a bit different than your approach so I wanted to ask what's your opinion about adding an extra argument called omit_defaults, and only omit defaults it present. I'm adding this because unlike your first patch, these aren't actually defaults when creating the objects (like ast.Module(body=[x]) != ast.Module(body=[x], type_ignores=[])) so doing anything other than looking to that representation would be different than the actual result. What're your opinions about this? (I'm submitting the initial version of the patch before doing a PR)
msg364326 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python triager) Date: 2020-03-16 14:24
Actually I have a better solution for this (which I hope to share really soon if it works.) I think we can do default value initialization for both Nones (with your patch) and lists, it requires a bit of extra work but  I think I can do it.
msg365987 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python triager) Date: 2020-04-08 14:28
Adding issue 39981 as a dependency.
History
Date User Action Args
2020-04-08 14:28:27BTaskayasetdependencies: + Default values for AST Nodes
messages: + msg365987
2020-03-16 14:24:16BTaskayasetmessages: + msg364326
2020-03-16 10:00:56BTaskayasetfiles: + ast.patch

messages: + msg364305
2020-03-09 22:07:54serhiy.storchakasetmessages: + msg363775
2020-03-09 21:51:24brett.cannonsetnosy: - brett.cannon
2020-03-08 08:56:35BTaskayasetmessages: + msg363642
2020-03-08 08:56:12BTaskayasetmessages: + msg363641
2020-03-08 08:33:57serhiy.storchakasetmessages: + msg363640
versions: - Python 3.8
2020-03-08 07:57:20serhiy.storchakasetpull_requests: + pull_request18200
2019-11-20 00:42:52levkivskyisetmessages: + msg357012
2019-11-19 00:10:29eamanusetmessages: + msg356935
2019-11-18 22:41:31BTaskayasetnosy: + BTaskaya

messages: + msg356925
versions: + Python 3.9
2019-03-16 06:25:33serhiy.storchakasetmessages: + msg338055
2019-03-15 20:54:42eamanusetmessages: + msg338033
2019-03-15 20:33:20levkivskyisetnosy: + levkivskyi
messages: + msg338030
2019-03-14 17:27:34brett.cannonsetmessages: + msg337955
2019-03-14 14:36:47eamanusetmessages: + msg337922
2019-03-14 14:35:08eamanusetkeywords: + patch
stage: patch review
pull_requests: + pull_request12299
2019-03-14 12:19:06eamanusetnosy: + eamanu
2019-03-14 09:48:05serhiy.storchakacreate