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
Improve / Clear ASDL generator #84708
Comments
|
You just read my thoughts. That's what I was going to do tonight. ;) |
:) Awesome. Well I'll plan to continue this; @pablogsal and I was discussing the idea of extracting _ast.AST subclasses to python level (which would save a lot of time for new additions). So my primary goal is clearing bunch of old stuff / making it extendable and clean. |
Currently working on implementing some items from this TODO https://github.com/eliben/asdl_parser/blob/master/TODO |
I was also thinking about getting rid of "string" nodes and just use Constant for those. Having the strings raw make the AST inconsistent because nodes that are 'strings' do not have line numbers and other metadata, making tools that need the source to report stuff more complex than it should. What are your opinions on this @serhiy.storchaka and @BTaskaya? |
I'm +1 on that (especially for finding import aliases, as we discussed earlier), if necessary precautions happen to ensure minimum breakage (from what I understand, it is something that will break everyone's code. Maybe we can implement custom Constant.__str__? and other different things to ensure it breaks small). |
"string" is only used for
For which of them a line number is meaningful and useful? |
Apologies, I was thinking of 'identifier', which is basically a PyObject representing a string. For instance, when parsing function calls like x = ast.parse("f(x=435)") the 'arg' attribute of the keyword is just the string 'x', without any metadata about it (like column offset and such). |
Oh, I confuse with it identifier. IMHO replacing identifier with Constant would infer position in some cases. |
A real world example would be tools like |
Next is using asdl_int_seq for all simple sum types, and not for only cmpop. Looks like it is hardcoded to use it, and I plan to refactor this in a way that might save some time in python implementation of classes (also it will solve this bug). For not conflicting, I'll wait GH 19968. |
These were all the cleanups on my checklist, so can close the issue now. I'll probably create other issues if something additional comes up |
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: