add type hints

[prandla]

Converting old docstring-based type hints to PEP 484 type hints. Current progress: all of cms/ except for cms/server/, and all of cmscommon/.

Notes:

• SQLAlchemy models are typed mostly correctly (required a few hacks, see diff of cms/db/base.py). SQLAlchemy queries are still untyped, requires upgrading to SQLAlchemy 2.0. It's also possible to manually type-hint the result of queries though.
• RPC method calls aren't type-checked. I couldn't figure out a way to do that without changing the method call syntax; I think it would be possible if RPC calls looked like other_service.rpc.method(args) (as opposed to the current other_service.method(args)).
• Minimum Python version is now 3.12 (for the new generic syntax). It would be pretty easy to port to python 3.11, but i don't want to go below that. I think this is not a huge issue, most distros have >=3.12 already, Debian is the last holdout and a new debian release should be happening in a few months.
• all of our jinja templates will forever be non-type-checked. this makes me sad :(

[veluca93]

@gollux is the python 3.12 minimum version requirement likely to be a problem for IOI?

[gollux]

This IOI will run on Ubuntu 2024.04.02 with Python 3.12.3.

However, I would prefer requiring at least 3.11, because that's in stable Debian, which is sort of the lower bound for all reasonable Linux distros ;)

What feature from 3.12 do you need?

[prandla]

What feature from 3.12 do you need?

the new type parameter syntax. i also wanted to use typing.override, but couldn't be bothered with it (there's a few functions where the return type depends on a boolean argument, using override allows typing them more precisely than a simple union).

(edit: brain fart, i meant overload, which was introduced before 3.12 and is actually fine to use)

it's okay, i'll just replace it with manual typing.TypeVar usage.

we might want to also run tests on the oldest version of python we want to support.

[prandla]

Tests now pass on python 3.11 aswell. I'll make a separate PR later to automatically test this on github actions, for now i'm testing on 3.11 locally.

[prandla]

some more notes:

for cmsranking and cmscontrib i tried to add type hints to all functions, even where they weren't present before. for cmstestsuite i only converted the old type hints to new-style ones.

there's a few modules where it would be possible to have much better static typing if the code was refactored a bit. currently i tried to keep the existing code untouched. (though i added a few asserts that should always be true, in cases where there's some complicated control flow and the type checker can't figure out that something is always not-None.)

the web server parts had less type hints than i expected; this is mostly because a lot of it is just passing parameters into jinja templates which are, again, completely untyped.

A lot of the remaining type errors are about sqlalchemy queries, which could be fixed by upgrading to sqlalchemy 2.0 (i think). I might try that at some point. The rest are in tornado/werkzeug/jinja, and as i understand, upgrading those is quite the ordeal. But I suppose it must still happen some day. And there's also a lot of type errors about missing None checks, most of them false positives, some of them quite certainly real :)

[veluca93]

LGTM.

Do you think it would make sense to add a CI step to check typing?

[prandla]

Do you think it would make sense to add a CI step to check typing?

There's still a lot of type errors, most of them not really relevant, so I think currently it would just be a lot of noise. But maybe someday in the future. Or we can try to enforce that PRs don't introduce new type errors, but I don't know if pyright or mypy support that.