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 skips strict mypy checks #247

Open
drnextgis opened this issue Jan 15, 2025 · 6 comments
Open

pre-commit skips strict mypy checks #247

drnextgis opened this issue Jan 15, 2025 · 6 comments

Comments

@drnextgis
Copy link
Contributor

According to the documentation, pre-commit runs mypy with only the --ignore-missing-imports flag. So if I'm not wrong the pyproject.toml file is not being used, which results in the strict = true setting in the [tool.mypy] section being ignored, causing pre-commit to bypass strict checks.

@gadomski
Copy link
Member

You sure? I just removed the additional dependencies, ran pre-commit run --all-files, and got a bunch of errors including:

tests/conftest.py:9: error: Untyped decorator makes function "asset_path" untyped  [misc]
tests/conftest.py:14: error: Untyped decorator makes function "item_path" untyped  [misc]
tests/conftest.py:19: error: Untyped decorator makes function "item_collection_path" untyped  [misc]
tests/conftest.py:26: error: Untyped decorator makes function "data_path" untyped  [misc]
tests/conftest.py:31: error: Untyped decorator makes function "item" untyped  [misc]
tests/conftest.py:36: error: Untyped decorator makes function "collection" untyped  [misc]
tests/conftest.py:41: error: Untyped decorator makes function "item_collection" untyped  [misc]
src/stac_asset/client.py:10: error: Library stubs not installed for "aiofiles"  [import-untyped]

...etc

@drnextgis
Copy link
Contributor Author

drnextgis commented Jan 15, 2025

What I'm saying is that pre-commit doesn't use the pyproject.toml file, so the result you’re seeing makes sense. Look:

➜ uv run pre-commit run --all-files
mypy.....................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed

but:

➜ uv run mypy src --config-file pyproject.toml
src/stac_asset/http_client.py:9: error: Skipping analyzing "aiohttp_oauth2_client.client": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/stac_asset/http_client.py:10: error: Skipping analyzing "aiohttp_oauth2_client.models.grant": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/stac_asset/http_client.py:10: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
src/stac_asset/http_client.py:69: error: Skipping analyzing "aiohttp_oauth2_client.grant.device_code": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/stac_asset/http_client.py:79: error: Skipping analyzing "aiohttp_oauth2_client.grant.authorization_code": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/stac_asset/http_client.py:91: error: Skipping analyzing "aiohttp_oauth2_client.grant.resource_owner_password_credentials": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/stac_asset/http_client.py:103: error: Skipping analyzing "aiohttp_oauth2_client.grant.client_credentials": module is installed, but missing library stubs or py.typed marker  [import-untyped]
src/stac_asset/http_client.py:133: error: Incompatible types in assignment (expression has type "ClientSession | RetryClient", variable has type "ClientSession")  [assignment]
Found 7 errors in 1 file (checked 16 source files)

@gadomski
Copy link
Member

Right on -- yeah, I'd be happy to mirror our pyproject.toml settings to the pre-commit config. To be honest, on other projects I'm moving away from pre-commit for exactly this reason, and instead just providing a scripts/lint and a scripts/format that runs in CI.

@drnextgis
Copy link
Contributor Author

Do you know if there's a way to configure mypy in the pre-commit to use the pyproject.toml file?

@gadomski
Copy link
Member

I don't see anything, and this SO answer (https://stackoverflow.com/a/74291151/732529) from the pre-commit author doesn't mention anything so my guess might be no.

I wonder if using system would find pyproject.toml?

@jkeifer
Copy link
Member

jkeifer commented Jan 15, 2025

See the example config in for cirrus: https://github.com/cirrus-geo/cirrus-geo/blob/main/.pre-commit-config.yaml. I can’t say I configure it perfectly, but the use of the “local” repo tells pre-commit there’s nothing to install, and in effect everything must be system. All the hook configs are then lifted directly, more or less, from each hooks repo, i.e., what would have been installed pulling the hook definition from some repo.

I absolutely hate pre-commit as a “package manager” because it is just another place code is getting downloaded and executed on my system. So I ensure all required hooks are installed as part of the dev requirements and configure them as local hooks so it doesn’t do any of that nonsense. It has the added benefit that things like mypy then operate as expected.

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

No branches or pull requests

3 participants