search

CarletonURocketry / ground-station

The ground station software used to interface with the ground station LoRa board and distribute signals from the rocket across websocket connections.
https://carletonurocketry.github.io/ground-station/
MIT License
7 stars 6 forks source link

Remove all type: ignore #87

Closed EliasJRH closed 8 months ago

EliasJRH commented 8 months ago

There are a lot of instances of # type: ignore in the codebase. While they don't need to get removed, it might be a good idea to look into why they're there and get rid of them.

linguini1 commented 8 months ago

These are for the Pyright linting and they're important to be there for situations where Pyright can't infer types itself.

EliasJRH commented 8 months ago

These are for the Pyright linting and they're important to be there for situations where Pyright can't infer types itself.

I understand, but there are a few instances of these that arise because we are avoiding fixing the types when they could be fixed. For example, in telemetry.py, the self.replay_input field has # type: ignore because we are casting a multiprocessing queue to a regular python queue. I believe these can be fixed

linguini1 commented 8 months ago

These are for the Pyright linting and they're important to be there for situations where Pyright can't infer types itself.

I understand, but there are a few instances of these that arise because we are avoiding fixing the types when they could be fixed. For example, in telemetry.py, the self.replay_input field has # type: ignore because we are casting a multiprocessing queue to a regular python queue. I believe these can be fixed

There's actually a really long issue for this where basically multiprocessing queues cannot be used as type annotations, only regular queues. So using the regular queues actually allow proper type hinting. Unfortunately it doesn't work flawlessly when constructing the new queues because Pyright still can't infer, hence the comments.

I lost way too many hours on this: https://github.com/python/cpython/issues/99509

If there's something else I'm not understanding about the comments please lmk but I'm keeping this issue closed for now as the # type: ignore comments that remain are specifically there for a reason

EliasJRH commented 8 months ago

Big L, Understood

linguini1 commented 8 months ago

Yeah pretty much sucks. If you are experimenting with them though and find some you can remove, just lmk and I'll reopen