Skip to content
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

Improvements to stubgenc #14564

Merged
merged 10 commits into from
Apr 15, 2023
Merged

Conversation

chadrik
Copy link
Contributor

@chadrik chadrik commented Jan 30, 2023

This bundles together a handful of improvements to stubgenc. Each one has its own commit, but it can be somewhat time consuming to get a PR reviewed and merged, so I'm putting them together for the sake of expediency. I'll break them up if it is preferred.

An overview of the main changes:

  • infer return types for known special methods: previously only argument types were inferred
  • check class docstrings for signatures in addition to __init__: shiboken binding generator is known to put constructor signatures in class docstrings rather than on __init__
  • use the list of analyzed modules to produce better imports: previously when given foo.Bar.Spangle, it was assumed that import foo.Bar should be added, however, it could be that Bar.Spangle refers to nested classes within the module foo.
  • when fixing up types, also process children of compound types
  • evaluate descriptors when getting members: necessary for shiboken bindings

@chadrik chadrik changed the title Improvemetns to stubgenc Improvements to stubgenc Jan 30, 2023
@chadrik
Copy link
Contributor Author

chadrik commented Jan 30, 2023

@hauntsaninja if you have some time it'd be great to get your eyes on this PR as well. thanks! There's just one little test left to fix.

@chadrik
Copy link
Contributor Author

chadrik commented Mar 11, 2023

@hauntsaninja do you think you'll have time to review this?

@JelleZijlstra JelleZijlstra self-requested a review March 11, 2023 03:33
mypy/stubgenc.py Outdated
continue
# First try to get the value via getattr. Some descriptors don't
# like calling their __get__ (see bug #1785), so fall back to
# looking in the __dict__.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems wrong; the current code discards cases where getattr() fails, it doesn't fall back to the value in __dict__.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. This was adapted from inspect.getmembers and I left the comment unaltered. This is fixed.

@@ -257,10 +314,12 @@ def generate_c_function_stub(
module: ModuleType,
name: str,
obj: object,
known_modules: list[str],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make the argument keyword-only here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

name = name[2:-2]
if name in ("float", "bool", "bytes", "int"):
return name
elif name in ("eq", "ne", "lt", "le", "gt", "ge", "contains"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't entirely safe as __eq__ and co may return arbitrary types, but it's probably good enough for stubgen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I added a comment here to this effect.

@JelleZijlstra
Copy link
Member

Also, the pybind11 test is broken: we lose # read-only comments and some fields get reordered.

chadrik added 9 commits April 14, 2023 16:58
Previously only argument types were inferred
This adds parity with the external rst signature generator.

Qt's shiboken binding generator is known to put constructor signatures in class docstrings.
Previously when given 'foo.bar.spangle', it was assumed that 'import foo.bar' should be added, however, it could be that 'bar.spangle' refers to nested objects (such as classes) within 'foo'.  This commit improves this inference by using the list of modules passed to stubgen.
simplify the body of generate_c_type_stub
…ureGenerator

This allows specialized SignatureGenerator subclasses to provide the type of self/cls, such as when a TypeVar is present
@chadrik chadrik force-pushed the stubgenc-improvements1 branch from ddf5918 to 197592e Compare April 15, 2023 02:11
@chadrik chadrik force-pushed the stubgenc-improvements1 branch from 197592e to 7ac691a Compare April 15, 2023 03:40
@chadrik
Copy link
Contributor Author

chadrik commented Apr 15, 2023

Also, the pybind11 test is broken: we lose # read-only comments and some fields get reordered.

I fixed this. The problem was that is_static_property only works when it is passed a descriptor object and so it was being circumvented by the evaluation of the descriptors in get_members -- which I added to get stubgen working with PySide methods. So I adjusted the code so that properties operate on the unevaluated attribute straight out of obj.__dict__ and other members use the result of the evaluated descriptor.

@chadrik
Copy link
Contributor Author

chadrik commented Apr 15, 2023

This should be all ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants