Skip to content
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

Fully type the codebase #1338

Open
C0rn3j opened this issue Dec 7, 2024 · 0 comments
Open

Fully type the codebase #1338

C0rn3j opened this issue Dec 7, 2024 · 0 comments
Labels
heavy Technical debt or previous design decisions make this hard to move help wanted

Comments

@C0rn3j
Copy link
Collaborator

C0rn3j commented Dec 7, 2024

Having the codebase fully typed would prevent all kinds of issues, by being able to catch any new ones introduced by changes and any existing ones currently being hidden by lack of types.
It also speeds up the development, especially for those unfamiliar with the codebase.

It lets us go from this:
image

To this:
image

Typing everything successfully relies on many things, and is a very tall order, but that does not mean we cannot strive to get there eventually, as we can work at this piece by piece.

Add types for Tauon's code

This is the easy part, and is what I've been doing bit by bit in PRs like #1337

Pick an untyped thing, track it down as best you can and ideally type up the entire chain leading to it, and hope that it is correct, if not, it usually gets revealed later when other things get their typing.

Typing tends to reveal bugs somewhat often in the current codebase, such as treating a variable as list[str] initially but later as a str somewhere else, which may or may not work depending on the code, but it should at the very least be cleaned up even if it technically works. Or retype such variables to expect both cases, but that usually leads to more complicated code.

One can grab a list via:

ruff check --config pyproject.toml ./src/tauon --output-format concise | awk '{first=$1; $1=""; print $0, first}' | sort | grep annotation

Move things to classes where reasonable

Some things should be moved to classes instead of having accesses to them by random indexes.

See #1311 which refactors things to use TauonQueueItem and TauonPlaylist classes, we might want to do this for things like colors, which are currently used mostly as a list of 3 or 4 ints.

LSPs like Pyright expect to run the wrong OS codepaths

Fixing #1318 should resolve it.

LSPs like Pyright fail to parse t_main.py

image

This is partially caused by:

  • t_main.py being too large - it's 49KLOC at the time of writing
  • The functions being too complex - there's a lot of conditionals that could potentially be improved
  • The LSP of choice, some can deal with complex things better than others

We need to split and de-complexify codebase to the point where LSPs don't time out on it - this is hinging on having types in the first place as refactoring anything without them is not a sane idea.

For example, variable playlist in the codebase is sometimes an object containing among others a playlist: list[int], and sometimes it is simply an int, so moving things around without knowing what they are will be guaranteed to break things.
This specific playlist example has been mostly already taken care of.

So the approach to type up the part you want to refactor (as a submodule) first is generally the go to idea.

Making sure we don't break things while fixing things up

It would be best if we added CI tests as we go, tracked in #1315

Some libraries we depend on do not have (full) typing support

Best resolution would be to convince, ideally through a PR, the library maintainer(s) to add or fix typing.
This may be a difficult approach, as some maintainers may be difficult about continuing maintaining EOL Python versions which cannot support typing; lowest supported version needs to be, I believe, 3.8, or 3.7 at worst.
It should be the latest supported version.

Arguing that people on EOL Python versions can just keep using the old version of the library is usually successful.

When libraries have types, they need to explicitly mark the codebase as typed via py.typed marker file - https://typing.readthedocs.io/en/latest/spec/distributing.html#packaging-typed-libraries

For the libraries where direct typing support is not welcome, we could look at contributing them to typeshed if they're not already there.

Problematic libraries:

@C0rn3j C0rn3j added help wanted heavy Technical debt or previous design decisions make this hard to move good first issue and removed good first issue labels Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
heavy Technical debt or previous design decisions make this hard to move help wanted
Projects
None yet
Development

No branches or pull requests

1 participant