Include Python type checking step in Precommit

[richard-to]

The most common CI failure for me is the python type checking one since it's not automatically run during pre-commits. This means, issues of this type usually get caught in CI. And then I have to manually run the python type checking script.

[wwwillchen]

I think it's a little heavy-handed to do it as a pre-commit because if I'm just modifying docs, there's not really a need to do python type-checking. Similar to TS type-checking, I think it should be done as part of the build process (and thus making it harder to forget during development) - I think this is how it's done in google3, however I'm not sure how to integrate pyright with our py_library build rules.

[richard-to]

I was thinking that the type checking step would run only on the python files that changed or if python files have been changed, whichever is easiest to implement. Is it not possible to make precommit selectively run steps? Or to pass in certain args to the script? I have not used precommit really before, so not sure if there are limitations.

[wwwillchen]

You can filter based on file type, example: https://github.com/google/mesop/blob/6a65aad6d433c633be3f4a73a2d700c374ff08b4/.pre-commit-config.yaml#L39

I'm still a bit unconvinced about adding it here because Python type-checking can be quite slow (sometimes, and potentially I've pyright misconfigured, it can take a very long time, >1 minute, it seems to just hang).

BTW are you using the pylance VS Code extension: https://marketplace.visualstudio.com/items?itemName=ms-python.vscode-pylance? I find that it's quite responsive and helps me catch most of the type issues before committing.

[wwwillchen]

OK, I figured out why it was super slow, it was because pyright was choking on the venv directories which I've fixed in: https://github.com/google/mesop/pull/821/commits/4583e6752b9bab592180a0109687424c9bd04962 - it takes ~4 seconds for me now, so I'm now not opposed to this :)

[richard-to]

I see. That's good to know.

In terms of VS Code set up, I did have pylance set up, but looks like for Type Checking Mode, I had it set to "off". I changed it to "standard" Or is there a different setting I should used there?

[wwwillchen]

@richard-to - I think the main setting is https://github.com/google/mesop/blob/8b3f176c35c3b81768f6e945b74ada1ddd56bcdc/.vscode/settings.json#L26 and setting up venv: https://google.github.io/mesop/internal/development/#venv - once you do this, you should have almost no more red squigglies