-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
builtins: do the TODO on compile() #9567
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
Conversation
optimize: int = -1, | ||
*, | ||
_feature_version: int = -1, | ||
) -> _ast.AST: ... |
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.
We can potentially be more precise here: if mode is "eval", the node must be an _ast.expr
.
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.
Would we have to duplicate all 8 overloads from ast.parse
, or does compile
only take "exec", "eval" or "single" for the mode
parameter?
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.
Yes, we would. ast.parse
just calls compile()
. I think it's fine to stick with AST
for simplicity.
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.
Though I think we'd only need four overloads because the second argument to compile()
is required, unlike ast.parse
.
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.
Hmm. Four overloads isn't that bad? But I don't have a strong opinion either way.
Diff from mypy_primer, showing the effect of this PR on open source code: sympy (https://github.com/sympy/sympy)
+ sympy/parsing/sympy_parser.py:1075: error: Incompatible types in assignment (expression has type "CodeType", variable has type "str") [assignment]
jinja (https://github.com/pallets/jinja)
+ src/jinja2/environment.py:704: error: Unused "type: ignore" comment
|
This is a side-effect of more precise types; previously mypy allowed the redefinition because compile() returned Any.
https://github.com/pallets/jinja/blob/main/src/jinja2/environment.py#L704 This fixes a false positive from mypy. |
No description provided.