Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-134733: fix: ast.dump( show_empty=True ) now displays allNone values#134743
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Hmmm... It seems as if the ast tests are failing. The tests want |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@hunterhogan The issue is that the tests haven't been updated to handle the kind="None" attr.
We could either drop that or update the tests.
I think I am missing something because the existing tests seem sparse. I made some tests, but I didn't finish integrating them. I don't really know what I am doing. I could use a little help, to be honest. |
| @@ -0,0 +1,1477 @@ | |||
| import ast | |||
| check_node( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What ischeck_node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
From the existing tests.
cpython/Lib/test/test_ast/test_ast.py
Lines 1511 to 1539 infbbbc10
| deftest_dump_show_empty(self): | |
| defcheck_node(node,empty,full,**kwargs): | |
| withself.subTest(show_empty=False): | |
| self.assertEqual( | |
| ast.dump(node,show_empty=False,**kwargs), | |
| empty, | |
| ) | |
| withself.subTest(show_empty=True): | |
| self.assertEqual( | |
| ast.dump(node,show_empty=True,**kwargs), | |
| full, | |
| ) | |
| defcheck_text(code,empty,full,**kwargs): | |
| check_node(ast.parse(code),empty,full,**kwargs) | |
| check_node( | |
| ast.arguments(), | |
| empty="arguments()", | |
| full="arguments(posonlyargs=[], args=[], kwonlyargs=[], kw_defaults=[], defaults=[])", | |
| ) | |
| check_node( | |
| # Corner case: there are no real `Name` instances with `id=''`: | |
| ast.Name(id='',ctx=ast.Load()), | |
| empty="Name(id='', ctx=Load())", | |
| full="Name(id='', ctx=Load())", | |
| ) | |
sharktide commentedMay 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'll see what I can do. Busy now will get to this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm going file by file.
In the news entry we don't need the entire second sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@hunterhogan You never definedcheck_node.
I'm writing one now. Also, could you please add me as a collaborator to your branch so I can try a few things?
@hunterhogan test_ast/cases.py test_ast/utils.py test_ast/test_ast.py Is it okay if I do this? My version of check_node needs a specific format which would require rewriting all your tests |
I was attempting to integrate with the existing tests. cpython/Lib/test/test_ast/test_ast.py Lines 1511 to 1539 infbbbc10
I'm in over my head. |
I am not currently capable of accomplishing this task. #134718 will probably resolve it, though. |
@hunterhogan We'll see. Thanks for opening this PR and giving it a shot though! |
Uh oh!
There was an error while loading.Please reload this page.
Ensure that ast.dump includes all None values when the show_empty flag is set to True.
Also, changed
iftoeliffor consistency.ast.dump( show_empty = True)does not show mostNonevalues. #134733