-
Notifications
You must be signed in to change notification settings - Fork 36
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
[WIP] stub generation (Reboot) #211
Draft
fcole90
wants to merge
50
commits into
pygobject:master
Choose a base branch
from
fcole90:fco/stub-generation
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This also retains the original constant value on the Constant class so that the stubbing can introspect its type. We could instead do the typing-annotation style formatting on the constant class, but this same logic is going to be needed for e.g., formatting function parameter types.
This also retains the full function signature on functions so that we can get the Python typing annotations for args and return values correct.
This makes the virtualenv-installed setup work.
This makes it a lot easier because we can just add a `typing` import to the top of the `.pyi` file and forget about everything else.
The problem here is that we need class/struct/etc. references within the module being annotated to not have the module prefix, because otherwise we just get unresolvable typing references. We also need all of these to be forward references (i.e., strings) unless we want to do some kind of dependency ordering, which I don't. I'm not a fan of doing this with a global, but the other solutions are much more complicated. Maybe we can make a stubber state class instead of this, but it's not obvious how much better that would be.
While there's dependency information in the namespace class, it's not reliable when we need to have all relevant modules in the current namespace for name resolution. The solution here is to constantly update a list of modules that have been referenced by current-module annotations, since that's what mypy, etc. are going to need. This change also means that we can't write import order until the end of the stubbing, so we're now keeping the stubs in a StringIO.
This doesn't cover everything, but now at least we get GType, which was causing a lot of issues.
Firstly, it's worth noting that this only ignores the typing error. Things that call these APIs will still be checked! The problem is that the checking may be incorrect because of the API itself. There's a long discussion about this and related Liskov issues at: python/mypy#1237 The short version is that this API is wrong-ish, but there's no mechanism within mypy to correctly annotate what's going on here. There *is* some of this around, because mypy treats __init__ and __new__ differently itself, but we can't apply that treatment to our constructors. This would be better if we actually knew which methods were constructors, instead of the name-based guessing here... but that's way too much complexity for me right now.
These are functionally identical anyway, and are *almost* the same as the generic class stubber.
These are special cases for some base types lacking introspection information. Since nothing else seems to have this issue other than low-level GObject types, we'll treat this as a weird case. This gets rid of mypy errors like: GObject.pyi:1416: error: Name 'NoneType' is not defined in module-level definitions for e.g., GBoxed, GPointer, etc.
This isn't a generic solution for shadowing... that would require actual parsing and tricks. However, in practice the risky shadowing here happens when e.g., a class implements def int(self) -> int: ... and everything breaks. This works around that in a somewhat-inelegant but basically-correct kind of way.
According to mypy, these aren't necessary in stub files.
They support common integer methods, so this is definitely reasonable.
flake8 is trying to do the right thing here, but in this instance we're dealing with getting an actual type, so the "use isinstance instead" error really isn't helpful or correct.
These aren't in the module requirements, and aren't installed by default on non-Debian-based systems.
The check for whether a record is private assumes a fixed child node structure that isn't the case in some environments. This patch tries to keep the spirit of the check by filtering out all known-unimportant child nodes and simply checking whether there's anything left. The expected unimportant nodes are: * empty text nodes containing formatting white-space * source-position elements that map the member to the original source file neither of which indicate that the member actually needs to be annotated.
This is needed for e.g., classes that inherit from both Atk.Object and Atk.Action, which both define methods like `get_description` with very different signatures.
GObject-derived widgets always accept keyword args for setting GObject properties. Ideally we'd check that the kwargs matched the object's properties, but for now we'll just accept all kwargs to avoid the type errors.
This is currently only for GObject, but there's several cases where backwards-compatible aliases have been provided.
Ideally this would come from the GI constructor information, but we don't maintain that through to the point of use here yet. This list should be able to be removed with some work in the future though.
Object.Property and Object.Signal are the main two missing parts for general use, and both require hand-written stub information. The Property stub here is probably wrong in various ways and could definitely be a lot better, but it's better than nothing.
Any progress on this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch is to continue the work done in #176.
I'm working on this in my spare time, so don't expect much.