mypy
mypy copied to clipboard
stubgenc: Introduce an object-oriented system for extracting function signatures
Description
Note: This change is part of a series of upcoming MRs to improve stubgenc.
stubgenc tries 3 approaches to infer signatures for a function. There are two problems with the current design:
- the logic for how these approaches are applied is somewhat convoluted, to the point that it's not even clear at first that there are 3 distinct approaches (from rst files, from docstrings, fallback guess).
- there's not a clear path for how a developer would create a new approach.
Here's the heart of the current logic (comments added for clarity):
if (
name in ("__new__", "__init__")
and name not in sigs
and class_name
and class_name in class_sigs
):
# get from rst (only works classes)
inferred: Optional[List[FunctionSig]] = [
FunctionSig(
name=name,
args=infer_arg_sig_from_anon_docstring(class_sigs[class_name]),
ret_type=ret_type,
)
]
else:
docstr = getattr(obj, '__doc__', None)
# get from docstring
inferred = infer_sig_from_docstring(docstr, name)
if inferred:
assert docstr is not None
if is_pybind11_overloaded_function_docstring(docstr, name):
del inferred[-1]
if not inferred:
if class_name and name not in sigs:
# fall back to using a list of known special methods
inferred = [FunctionSig(name, args=infer_method_sig(name, self_var),
ret_type=ret_type)]
else:
# fall back to a catchall signature
inferred = [FunctionSig(name=name,
args=infer_arg_sig_from_anon_docstring(
sigs.get(name, '(*args, **kwargs)')),
ret_type=ret_type)]
elif class_name and self_var:
# why do we only fix self var for docstring signatures and not rst signatures?
args = inferred[0].args
if not args or args[0].name != self_var:
args.insert(0, ArgSig(name=self_var))
And after my changes:
inferred: Optional[List[FunctionSig]] = None
if class_name:
# method:
assert self_var is not None, "self_var should be provided for methods"
for sig_gen in sig_generators:
inferred = sig_gen.get_method_sig(obj, module.__name__, class_name, name, self_var)
if inferred:
args = inferred[0].args
if not args or args[0].name != self_var:
args.insert(0, ArgSig(name=self_var))
break
else:
# function:
for sig_gen in sig_generators:
inferred = sig_gen.get_function_sig(obj, module.__name__, name)
if inferred:
break
This clarifies the logic and opens up the possibility for future expansion and customization.
Test Plan
This MR has two commits:
- implement the object-oriented inference system: this change is designed to preserve the current behavior so required no changes to test behavior
- fix a bug where
@classmethodand self-var fixes were not applied to every overload. tests have been updated and a new test added to reflect the change.
Hi, any thoughts on this?
Hey everyone, it would be great to get some feedback on this PR.
@ilevkivskyi Sorry to flag you directly here, but I'm not sure how to get someone to look at my PR. Can you help me out? This is the first of several PRs that I'd like to make to improve stubgen and it's been 3 weeks without any response, so at this rate it's going to take awhile!
Sorry, this is volunteer run and people may just not have time to look at this. As for reviewers, I guess @hauntsaninja may be a good person to look at this.
Sorry, this is volunteer run and people may just not have time to look at this.
I completely understand, and I appreciate all of your efforts! Just looking for anyway that I can get a reviewer to help me get this through. I've got what I think are some great contributions, that start with this MR. I've used the complete set of changes to make a really nice set of stubs for PySide and I've got more to come.
Thanks, this looks good! Your PR description made this easy to review. Left one nit, tag me once you've made the change (assuming you agree).
Thanks for helping out. I've responded to your suggestion.
(I'd say I'm mostly casually acquainted with stubgen and don't feel any great sense of ownership over it, so I'm not going to make promises... but if you ever have another PR that gets stalled more than a month, let me know)
Thanks for the offer!
Thanks! Please don't force push to PRs after they've been reviewed. It makes reviewing the delta much harder, especially so when the force push includes a rebase. We squash commits on merge. If you want to test against latest master, hit the "Update branch" button in the UI.