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

Pre-commit does not report MyPy errors #1665

Open
e10e3 opened this issue Jan 9, 2025 · 4 comments · May be fixed by #1666
Open

Pre-commit does not report MyPy errors #1665

e10e3 opened this issue Jan 9, 2025 · 4 comments · May be fixed by #1666

Comments

@e10e3
Copy link
Contributor

e10e3 commented Jan 9, 2025

Versions

River version: 0.22.0
Python version: 3.12.8
Operating system: macOS 15.2

Describe the bug

Pre-commit only runs its checks on changed files.
If a configuration was changed in a way that should impact the CI's output but no code file was changed, Pre-commit will not report any error.
(Surprisingly, the option --all-files does not help here).

The CI for the most recent commit reports no typing error, but running MyPy manually does.

In this case, here is what I suspect happened:
In commit e069b67, MyPy's version was bumped to 1.13.0 (see the diff here).
This new version of MyPy enabled new checks.
But because some Python files were not touched since, the CI never reported the new errors.

Steps/code to reproduce

  1. Checkout the main branch (currently at commit 5531ae5)
  2. Run pre-commit run -a
  3. See there are no errors
  4. Run mypy river
  5. 5 errors are reported

Expected behaviour

Pre-commit should report the MyPy errors, and not pass the MyPy hook.
Alternatively: The CI should report all errors, whether they are in modified files or not.

@MaxHalford
Copy link
Member

Alternatively: The CI should report all errors, whether they are in modified files or not.

I think this is what we should strive for.

@e10e3
Copy link
Contributor Author

e10e3 commented Jan 9, 2025

Alternatively: The CI should report all errors, whether they are in modified files or not.

I think this is what we should strive for.

I agree. I mentioned Pre-commit because this is the tool we currently use, but this objective would apply to the CI in general, even if another system is used in the end.

@MaxHalford
Copy link
Member

By the way we've started using pyright at Carbonfact and it's pretty good!

@aaelony
Copy link

aaelony commented Jan 9, 2025

ruff might also be useful.

@e10e3 e10e3 linked a pull request Jan 16, 2025 that will close this issue
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 a pull request may close this issue.

3 participants