-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
Make codebase work with basic mypy usage #1084
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All my comments are for variables not having a value assigned where before it would be None
. Not assigning a value causes an AttributeError
for me, am I missing something?
>>> class Test(object):
__test__: str
>>> Test.__test__
Traceback (most recent call last):
File "<pyshell#13>", line 1, in <module>
Test.__test__
AttributeError: type object 'Test' has no attribute '__test__'
Below works
from typing import Optional
class Test(object):
__test__: Optional[str] = None
__icon__ = None | ||
__priority__ = None | ||
__icon__: str | ||
__priority__: int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional[str] = None
@@ -16,7 +16,7 @@ class NMConnectionError(Exception): | |||
|
|||
|
|||
class NMConnectionBase(object): | |||
conntype = None | |||
conntype: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
conntype: Optional[str] = None
__description__ = None | ||
__author__ = None | ||
__description__: str | ||
__author__: str |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional[str] = None
|
||
__unloadable__ = True | ||
__autoload__ = True | ||
|
||
__instance__ = None | ||
|
||
__gsettings__ = None | ||
__gsettings__: "GSettings" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__gsettings__: Optional["Gsettings"] = None
instances = [] | ||
__plugin_info__ = None | ||
instances: List["ServicePlugin"] = [] | ||
__plugin_info__: Tuple[str, str] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__plugin_info__: Optional[Tuple[str, str]] = None
|
||
class GSettings(TypedDict): | ||
schema: str | ||
path: None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path: Optional[str] = None
Also do you have typehints for PyGobject? I tried [1] but they are incomplete so mypy is reporting lots of problems for me. [2] looks promising but is not merged. [1] https://github.com/pygobject/pygobject-stubs |
blueman/bluez/Base.py
Outdated
@@ -47,7 +49,7 @@ class Base(Gio.DBusProxy, metaclass=BaseMeta): | |||
__name = 'org.bluez' | |||
__bus_type = Gio.BusType.SYSTEM | |||
|
|||
__gsignals__ = { | |||
__gsignals__: Dict[str, Tuple[Any, GObject.SignalFlags, Tuple]] = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first Tuple starts with GObject.SignalFlags not Any, next is None and then a tuple. For the second tuple we know the first is a str and we know what types to expect from dbus properties which we can represent as a Union. So I think it should be the below which works for Base.
Dict[str, Tuple[GObject.SignalFlags, None, Tuple[str, Union[str, int, bool, List[str], Dict[str, GLib.Variant], Dict[int, GLib.Variant]], str]]]]
.
But not for subclasses that add their own signals so they will need their own type hints.
Where do you get such an exception? The idea is that those properties are not optional, so they cannot be
Nope. Not much more than what you know. |
The inner tuple is actually a tuple of an arbitrary number of GObject.GTypes that are always GObject.TYPE_PYOBJECT and a maximum of three in our case, so a precise type would be `Union[Tuple[()], Tuple[Literal[GObject.TYPE_PYOBJECT]], Tuple[Literal[GObject.TYPE_PYOBJECT], Literal[GObject.TYPE_PYOBJECT]], Tuple[Literal[GObject.TYPE_PYOBJECT], Literal[GObject.TYPE_PYOBJECT], Literal[GObject.TYPE_PYOBJECT]]]`, however as we do not have useful gi stubs no real checks get applied anyway, so we can keep it simple.
Not in blueman as at the moment all subclasses define them before using. If these are not optional how are you going to enforce the plugin defining them without crashing? Or is the plugin crashing the way 😄. The Optional for the path in gsettings is really optional so it needs fixing. |
mypy -p blueman --ignore-missing-imports
reports no findings with these changes (mypy 0.670 and 0.720).