Add typings to some parts of the code #96
Conversation
|
You should be able to drop the format everything with ruff commit, I've taken in most of the changes that Ruff recommended, still need to later mark the areas I left out so it does not touch it on the next run. Hopefully this does not make it too annoying to drop the commit and rebase. I'm planning to also run |
|
Shot myself in the foot with git a bit, everything should be a-ok now |
MangoTCF
changed the title
|
Should I infer and add a complete, really big, type to the config in |
|
We can always make it more specific later if desired, Any is probably fine there |
|
Also we really need to migrate away from PyGObject as it at the very least messes with typecheckers And, like, the whole GTK5 release = GTK3 EOL thing I propose Qt 6 as it looks like thay are moving in the right direction |
|
|
||
| def __init__(self): | ||
| self.in_use = False | ||
| super().__init__() |
There was a problem hiding this comment.
Do we really need to init the super class here?
There was a problem hiding this comment.
Linter complains :) It's better to do that for code consistency etc.
| # why not move size to be the second argument then? | ||
| control_with, size = DEFAULT, control_with |
There was a problem hiding this comment.
It's a cursed shorthand.
>>> x, y = 1, 2
>>> x, y = 3, x
>>> print(x, y)
3 1There was a problem hiding this comment.
Yeah, i get it, but why do
def f(a, b, c=DEFAULT, ..., shorthand_notation_thing=0):
if one can do
def f(a, b, shorthand_notation_thing=0, c=DEFAULT, ...):
?
|
|
||
| def whole(self, mapper, x, y, what): | ||
| @override | ||
| def whole(self, mapper, x: int | float, y: int | float, what: str, *_params): |
There was a problem hiding this comment.
My linter says that this method "overrides in an incompatible way". There's some more errors like this, but I'd need some more experience with the codebase to be sure how to fix them
There was a problem hiding this comment.
Basically, the signatures of the methods should be compatible as per the docs which IMHO is a good idea
|
I've unresolved some things you've set as resolved without a change/answer and added some more. I think besides the currently open conversations, this is LGTM, so please let's get this PR in before expanding it further, it's getting hard to review, thanks! |
|
Also to avoid having to do the whole rebasing thing on the next PR you could run |
|
@C0rn3j could you please look through the open conversations and affirm that everything is OK and my reasons for doing the things I did are valid? |
|
Did you perhaps Start a review but never sent it in? I see no comments. |
| class Keys(IntEnum): | ||
| """Keys enum contains all keys and button from linux/uinput.h (KEY_* BTN_*).""" | ||
|
|
||
| # TODO: Put these back when minimum Python version is... 3.10? https://github.com/C0rn3j/sc-controller/issues/24 |
There was a problem hiding this comment.
Also, what's the stance with these TODOs? 3.09 is EOL as per this
|
|
||
| def whole(self, mapper, x, y, what): | ||
| @override | ||
| def whole(self, mapper, x: int | float, y: int | float, what: str, *_params): |
There was a problem hiding this comment.
Basically, the signatures of the methods should be compatible as per the docs which IMHO is a good idea
|
Yup, you're completely right -_- i need to learn how to interface with all this better |
|
@C0rn3j Any updates? Or did i do something wrong again? :p |
2 participants
This PR is part 1 of many.
Files modified by me are:
Other files are reformatted via ruff as per the configuration in
pyproject.tomlAs open as i could be to constructive criticism, as this is my first more or less serious PR.