Add stubs/gi - #1353
Conversation
|
I remember suggesting this before.. I am all for creating stubs for the pygobject parts we use and maintain them until there are reliable ones we can install. |
8b1d285 to
4f6b37d
Compare
|
(This is based on #1341 for more complete type hints now) The path through GLib, GObject and Gio was surprisingly quick with a "pgi-docgen stubs, then edit" approach. #1304 (comment) is currently blocking the only error that is left for them. 😃 |
e5e9274 to
0597b9e
Compare
To add GI repository stubs, we need a gi/repository structure, so we also need to add an __init__.pyi and a types.pyi that covers what we use.
super() does not have __del__ and this method is unused anyway.
6b100bc to
35ff52f
Compare
|
😱 😱 😱 |
|
I'm a little sad that I found just one little bug that could actually show up (55792c5). On the other side... pretty cool! 😃 |
|
What, only one, you must have made a mistake 😆. This is going to take about of time to go through 😱. |
|
I tried to keep a clean commit history, in case you did not realize. There's obviously no need to review the generation commits. 🙂 |
|
Yea, my plan is to skim through the generated stubs but look closely at the modification that were done. Gtk3 is not going to change much but this will have to be redone when we move to Gtk4. |
|
I actually thought about removing all stuff that we do not use from the generated stubs as there are probably a lot of falsehoods in there that might lead to confusion if somebody makes the mistake to rely on the them (e.g. callback types are generally defined to take an optional object as user data while they usually take a variable amount of arguments; if one implements that signature as it is defined it would be wrong to pass zero or more than one user data argument but would be perfectly fine from the stubs perspective). Not sure how much work that would be, though, and one would have to restore the generated stuff as a basis when using additional things. |
infirit
left a comment
infirit
left a comment
There was a problem hiding this comment.
I see _get_widget copy/pasted all over the place. My suggestion would be to subclass Gtk.Builder, override get_object and move the instance checking in there. Alternatively which may be cleaner, add a new method get_widget.
As for the obviously wrong annotation in the stubs. Not sure, but we can take the approach that we fix them when it causes problems.
I still hate it with a passion how verbose and unreadable some of the annotations make function and classes. Oh well 🤷
|
|
||
| self.hide() | ||
| if self.window is not None: | ||
| self.window.hide() |
There was a problem hiding this comment.
I rather see we don't hide and pass the window to the ErrorDialog as parent.
| super().__init__(parent=None, flags=0, type=Gtk.MessageType.QUESTION, | ||
| buttons=Gtk.ButtonsType.NONE, message_format=None) | ||
| super().__init__(parent=None, type=Gtk.MessageType.QUESTION, | ||
| buttons=Gtk.ButtonsType.NONE, text=None) |
There was a problem hiding this comment.
Either do text=summary or just drop text=None.
Ideally we pass in all the properties we set later on (like text) as keyword arguments here.
There was a problem hiding this comment.
I was a little bit confused here. text defaults to "", not NULL, see https://developer.gnome.org/gtk3/stable/GtkMessageDialog.html#GtkMessageDialog--text, so I think text=None should actually be something else than the default but I did not really look into it.
|
|
||
| if is_paired: | ||
| icon_info = self.get_icon_info("dialog-password", 16, False) | ||
| _icon_info = self.get_icon_info("dialog-password", 16, False) |
There was a problem hiding this comment.
What is the point of the new variable? Below does the same, right?
icon_info = self.get_icon_info("dialog-password", 16, False)
assert icon_info is not NoneThere was a problem hiding this comment.
icon_info is already defined as non-optional in the method arguments, so we cannot assign the optional value from get_icon_info to it, but I could actually save icon_info = _icon_info as I could just use _icon_info.load_surface after the assertion directly.
| icon_info = _icon_info | ||
| trusted_surface = icon_info.load_surface(window) | ||
| assert isinstance(target, cairo.ImageSurface) | ||
| assert isinstance(trusted_surface, cairo.ImageSurface) |
There was a problem hiding this comment.
This should be fixed in the stub that load_surface return an instance of cairo.Surface. Or just force it in the stub to be an cairo.ImageSurface as I doubt we'll ever see anything else.
There was a problem hiding this comment.
This should be fixed in the stub that load_surface return an instance of cairo.Surface.
What do you mean? I think that's the case there, but we expect that Surface to be an ImageSurface which is not guaranteed.
| def _on_leave(self, _evbox: Gtk.EventBox, _event: Gdk.Event) -> bool: | ||
| assert self.Blueman.window is not None | ||
| self.Blueman.window.get_window().set_cursor(None) | ||
| self._get_window().set_cursor(None) |
There was a problem hiding this comment.
If we are going to add a helper function let's move it to Blueman and use it everywhere we use Blueman.window probably getting rid of a few asserts in the process.
|
Apologies it took so long. I have been busy and my motivation was very low to work on blueman. |
2 participants
Opening the GI repository stubs discussion.
First off: This will be a long way.
We need a
gi.repositorystructure to be able to use gir stubs. Unfortunately that means that we need a completegistructure but it's rather simple for us as all we need isgi.require_versionandgi.types.GObjectMeta. (Adding it already finds two additional typing issues 🙈)Regarding stub generation I'm using https://github.com/cschramm/pgi-docgen/tree/stub-generation which is currently just pygobject/pgi-docgen@12c9dbb merged with kaiw/pgi-docgen@992976e and a Pipfile for my convenience (the dependencies are otherwise just mangled into an (incomplete) setup script with pip calls or a Dockerfile).
One major problem with the currently existing stub generation is that it only considers GIR files and completely neglects both
gi.overridesand_gi, leading to a lot of typing errors regarding basic stuff likeGLib.timeout_addandGObject.connectand https://gitlab.gnome.org/GNOME/pygobject/-/tree/master/gi/overrides is huge.When I now generate GLib stubs with
pipenv run ./pgi-docgen stubs $BLUMEAN_SRC/stubs/gi/repository/ GLib-2.0runningMYPYPATH=stubs mypy -p blueman --ignore-missing-imports --warn-unused-configs --disallow-any-generics --disallow-incomplete-defs --check-untyped-defs --disallow-untyped-decorators --no-implicit-optional --warn-redundant-casts --warn-unused-ignores --warn-return-any --no-implicit-reexport --strict-equalitygives me ~200 errors (including a couple of errors in the pyi files themselves, mainly unused ignores). With Gtk and GdkX11 in place, it's ~1650 (side note: that's some more than what I get with pygobject-stubs).I found the following paths if we want to iterate one module after another (and that makes a lot of sense to me):
(mainly GLib -> GObject -> Gio -> Gdk -> Gtk for us)
pgi-docgen stubs do not even seem ready to add overrides e.g. for
GLib.timeout_addas there ispgidocgen/stuboverrides.pyto add stuff to modules and classes, but it cannot be used to actually override stuff. E.g. if I just add (overloading) definitions fortimeout_addto theGLibnamespace, the actual implementation that still gets generated below them will not match / replace them again and to be correct there, we would probably even have to remove it anyway as that signature is not valid from Python due togi.overrides. I'm not sure if it makes sense to work on pgi-docgen stubs to make it capable of actually overriding stuff so that it generates useful stubs or - much easier but feels kinda selfish - generate useless stubs once and then adapt them to reality.