Skip to content

chore: make pyright run in strict mode and add mypy #78

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

Merged
merged 4 commits into from
Apr 4, 2025

Conversation

serramatutu
Copy link
Collaborator

@serramatutu serramatutu commented Apr 3, 2025

Summary

This PR upgrade pyright typeCheckingMode from standard to strict, and it also adds mypy typechecking to the pre-commit hooks and to CI.

This is an attempt to make sure our type definitions are very sound, and will hopefully catch some bugs in the future.

There were some exceptions where I had to resort to # type: ignore.

Most of them are due to incomplete type annotations on third party libraries, which triggers the reportUnknownArgumentType or reportUnknownVariable checks from pyright (cough cough pyarrow).

Some other exceptions are due to there being to way to type things like Unpack[T] where T is not a concrete TypedDict and is instead a TypeVar that is bound to a TypedDict. In such cases, I added comments explaining why the ignore is necessary. Yes, I could rewrite the whole API to make it fully type-able, but that would also make the code a lot more verbose probably.

You can review commit by commit.

This commit introduces mypy to our pre-commit hooks and CI. This should
increase the amount of typing bugs we catch.
@serramatutu serramatutu requested a review from a team as a code owner April 3, 2025 14:35
@serramatutu serramatutu force-pushed the serramatutu/mypy branch 2 times, most recently from 38c4901 to e716d78 Compare April 3, 2025 18:24
This commit makes both pyright and mypy operate on the strictest
setting. I had to change some code around or add type ignores in cases
where it's being too strict or when the stubs for dependencies are not
precise enough.

I only did this for the `dbtsl` package in this commit. I'll refactor
the tests in the next commit.
This commit changes some of our tests to work with stricter pyright
settings. I made the checks for tests a little less strict than the real
`dbtsl` tests since our tests sometimes inject wrong types into the API
to purposefully induce errors, or mocks stuff that's really hard to
type.
@serramatutu serramatutu merged commit c56ac1a into main Apr 4, 2025
4 checks passed
@serramatutu serramatutu deleted the serramatutu/mypy branch April 4, 2025 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants