update pre-commit-config.yaml #3070
Conversation
| - torchmetrics>=1.6.2 | ||
| - torchvision>=0.18 | ||
| exclude: (build|data|dist|logo|logs|output)/ | ||
| exclude: (build|data|dist|logo|logs|output|docs/user/metrics)/ |
There was a problem hiding this comment.
Any reason why docs/user/metrics needs to be excluded? I think it's passing.
There was a problem hiding this comment.
It wasn't passing mypy for me locally. I can paste the errors.
There was a problem hiding this comment.
Please do. It's passing in CI. Maybe the min version of mypy in pre-commit needs to be bumped?
There was a problem hiding this comment.
Mainly the errors were around not having request mypy stubs. But maybe it's just a local thing
There was a problem hiding this comment.
Ah, that's something we set in pyproject.toml:
ignore_missing_imports = true
but pre-commit refuses to support pyproject.toml so we have to duplicate every single flag. Basically, there's a flag for this. Technically we should also add --strict, may require a couple overrides though.
There was a problem hiding this comment.
So pre-commit creates a separate environment to run the checks. Hence why the additional dependencies are needed. These deps are installed in that environment and the checks are run.
So the request errors should be resolved by adding types-requests to mypy's additional_dependencies.
There was a problem hiding this comment.
We should probably add a bunch of stub packages as testing deps. I wish these were within the requests source code, but at least someone wrote a stub.
|
I would like to keep pre-commit matching direct invocation of mypy. So we either ignore missing imports in pre-commit like we do in mypy, or we add type stubs to both. |
cb36125 to
619b072
Compare
|
This is a minor thing that we should just merge |
|
Not user facing so doesn't matter which release it goes into. Would like to sort out the type stubs stuff separately from updating dependencies here. |
No description provided.