GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — gh-104050: Annotate more Argument Clinic DSLParser state methods by erlend-aasland · Pull Request #106376 · python/cpython Conversation
|
|
@AlexWaygood, can you help me with the last bits on this PR? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Taking a look now :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
This diff fixes the first two mypy errors: --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4846,7 +4846,9 @@ def state_parameter(self, line: str | None) -> None: if not module: fail("Function " + self.function.name + " has an invalid parameter declaration:\n\t" + line) - function_args = module.body[0].args + function = module.body[0] + assert isinstance(function, ast.FunctionDef) + function_args = function.args if len(function_args.args) > 1: fail("Function " + self.function.name + " has an invalid parameter declaration (comma?):\n\t" + line) @@ -4931,7 +4933,9 @@ def bad_node(self, node): if bad: fail("Unsupported expression as default value: " + repr(default)) - expr = module.body[0].value + assignment = module.body[0] + assert isinstance(assignment, ast.Assign) + expr = assignment.value # mild hack: explicitly support NULL as a default value c_default: str | None if isinstance(expr, ast.Name) and expr.id == 'NULL': In both cases, mypy is flagging that, in principle, an cpython/Tools/clinic/clinic.py Lines 4849 to 4850 in 71b4044
For the second one, the source code that we're parsing is generated here: cpython/Tools/clinic/clinic.py Lines 4902 to 4904 in 71b4044
So we can be 100% confident that in the first case, This diff fixes the last mypy error: --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -4970,7 +4974,7 @@ def bad_node(self, node): else: value = ast.literal_eval(expr) py_default = repr(value) - if isinstance(value, (bool, None.__class__)): + if isinstance(value, (bool, NoneType)): c_default = "Py_" + py_default elif isinstance(value, str): c_default = c_repr(value) Type checkers do a lot of special casing around |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Of course, the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
No problem at all! 🚀 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Off-topic: this function is way too large and complicated, and desperately in need of modernisation as well. I had a go a while back (main...AlexWaygood:cpython:clinic-asts), but some tests were failing... that branch is also quite out of date now :-) And test coverage was poor, IIRC.
But maybe I'll get back to it at some point...
Sorry, something went wrong.
There was an error while loading. Please reload this page.
* main: pythongh-106368: Increase Argument Clinic test coverage (python#106389) pythongh-106320: Fix _PyImport_GetModuleAttr() declaration (python#106386) pythongh-106368: Harden Argument Clinic parser tests (python#106384) pythongh-106320: Remove private _PyImport C API functions (python#106383) pythongh-86085: Remove _PyCodec_Forget() declaration (python#106377) pythongh-106320: Remove more private _PyUnicode C API functions (python#106382) pythongh-104050: Annotate more Argument Clinic DSLParser state methods (python#106376) pythongh-106368: Clean up Argument Clinic tests (python#106373)
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.