Repository navigation
Make ast.dump() not output optional default fields #80468
Description
| BPO | 36287 |
|---|---|
| Nosy | @benjaminp, @serhiy-storchaka, @1st1, @ilevkivskyi, @eamanu, @isidentical |
| PRs | |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2019-03-14.09:48:05.485>
labels = ['type-feature', 'library', '3.9']
title = 'Make ast.dump() not output optional default fields'
updated_at = <Date 2020-04-08.14:28:27.407>
user = 'https://github.com/serhiy-storchaka'bugs.python.org fields:
activity = <Date 2020-04-08.14:28:27.407>
actor = 'BTaskaya'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2019-03-14.09:48:05.485>
creator = 'serhiy.storchaka'
dependencies = ['39981']
files = ['48975']
hgrepos = []
issue_num = 36287
keywords = ['patch']
message_count = 16.0
messages = ['337907', '337922', '337955', '338030', '338033', '338055', '356925', '356935', '357012', '363640', '363641', '363642', '363775', '364305', '364326', '365987']
nosy_count = 6.0
nosy_names = ['benjamin.peterson', 'serhiy.storchaka', 'yselivanov', 'levkivskyi', 'eamanu', 'BTaskaya']
pr_nums = ['12328', '18843']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue36287'
versions = ['Python 3.9']
Activity
serhiy-storchaka commented on Mar 14, 2019
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=[])"eamanu commented on Mar 14, 2019
Hi @serhiy.storchaka,
I send a patch to Github to review. Let me know if is necessary unittest.
Regards
brettcannon commented on Mar 14, 2019
@eamanu tests are basically always necessary. :)
ilevkivskyi commented on Mar 15, 2019
We can probably also skip type_ignores list if it is empty (which will be the case in 99% situations).
eamanu commented on Mar 15, 2019
Maybe we can ignore None and [] ?
serhiy-storchaka commented on Mar 16, 2019
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:
For Raise(cause=X) it outputs a string which is evaluated to Raise(exc=X).
isidentical commented on Nov 18, 2019
@eamanu are you still interested in this issue? (bumped to 3.9)
eamanu commented on Nov 19, 2019
Hi,
Sorry sincerely I forgot this issue, if there are not any objection I can continue it.
ilevkivskyi commented on Nov 20, 2019
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).
serhiy-storchaka commented on Mar 8, 2020
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.
isidentical commented on Mar 8, 2020
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.
isidentical commented on Mar 8, 2020
Related issue: bpo-39638
serhiy-storchaka commented on Mar 9, 2020
New changeset b7e9525 by Serhiy Storchaka in branch 'master':
bpo-36287: Make ast.dump() not output optional fields and attributes with default values. (GH-18843)
b7e9525
isidentical commented on Mar 16, 2020
@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)
isidentical commented on Mar 16, 2020
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.
isidentical commented on Apr 8, 2020
Adding bpo-39981 as a dependency.
furkanonder commented on May 11, 2023
@serhiy-storchaka The issue seems to have been solved. I think we can close the problem.