New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make ast.dump() not output optional default fields #80468
Comments
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=[])" |
Hi @serhiy.storchaka, I send a patch to Github to review. Let me know if is necessary unittest. Regards |
@eamanu tests are basically always necessary. :) |
We can probably also skip |
Maybe we can ignore None and [] ? |
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). |
@eamanu are you still interested in this issue? (bumped to 3.9) |
Hi, Sorry sincerely I forgot this issue, if there are not any objection I can continue it. |
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). |
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. |
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. |
Related issue: bpo-39638 |
@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) |
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. |
Adding bpo-39981 as a dependency. |
@serhiy-storchaka The issue seems to have been solved. I think we can close the problem. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: