patri9ck / a2ln-server

A way to display Android phone notifications on Linux (Server)
GNU General Public License v3.0
98 stars 8 forks source link

Fix a few type annotations for mypy #29

Closed Jan9103 closed 1 year ago

Jan9103 commented 1 year ago

Fixed a few issues with type annotations to make it (more) mypy compatible.

it is still not 100% mypy compatible since some of the used libraries are not annotated and therefore not everything can be checked.

patri9ck commented 1 year ago

Normally, I also prefer languages with static typing and null safety. mypy tries to add those feature to Python - a dynamic typed language with no null safety. In my opinion, that results in ugly syntax and badly readable code. Additionally, for a project of this size, dynamic typing and missing null safety will probably not result in big problems as compared to a big project.

Jan9103 commented 1 year ago

you can add null-safety via mypy by activating the strict mode

The reason for this MR is the existence of (according to [pep484}(https://peps.python.org/pep-0484/) wrong) type annotations in the current code. my editor (vim with a bunch of plugins) as well as many other setups automatically use mypy for completion, linting, etc as soon as a single type annotation is present.

patri9ck commented 1 year ago

Alright, I have been using PyCharm where it is fine but I see the problem.

patri9ck commented 1 year ago

@Jan9103 I made some more types "optional". Does it look fine to you? I tested some functions and at least there weren't any errors.

Jan9103 commented 1 year ago

the changes seem to be valid.

A issue mypy pointed out:
PairingServer.notification_server is optional and L310 uses self.notification_server.port without first checking if its None -> there might be a issue if you run it with --no-notification-server

patri9ck commented 1 year ago

This should fix it.