-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Typing the ast.AST subclass constructors #8378
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
Comments
I'd be in favour of adding |
Thoughts on the |
I would prefer explicit |
Invoking the constructors seems fragile to me. For example, if you instantiate
You can even make an empty module that doesn't have any attributes:
So code that instantiates AST nodes will break every time a new attribute is added. Maybe it means that you shouldn't do this and the type checker should error if you do this, or maybe it just means that type checking is unusually important here. |
@Akuli I interpret it as type checking being particularly important for this module, which is why I raised this issue. imo ideally the actual constructor would validate the arguments too, but it doesn't seem to do that, and I don't want to touch the C code that provides the constructor, so I'm instead aiming at a type annotated constructor. |
We currently have static types for the fields of most (all?)
ast
classes, but none of these have typed constructors, e.g.:typeshed/stdlib/_ast.pyi
Lines 79 to 86 in 62cde01
I think technically this might be because none of these subclasses actually have a unique constructor, but that shouldn't stop us from typing each of the constructors, since in practise the constructor arguments must correspond to the class fields.
Before I do this, though, I'm firstly wondering if an easy solution here would be to apply the
dataclass_transform
decorator (note: not the same as thedataclass
decorator). This would simply tell the type checker that all the class fields can and should be provided in the constructor, which is broadly correct. However I don't have a deep understanding of how theast.AST
constructor works, so this might not be the correct behaviour. ForClassDef
this might look like:If this isn't sufficient, I propose that we simply add an
__init__()
stub to each subclass. For instance, for theClassDef
above, this might look like:The text was updated successfully, but these errors were encountered: