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

Move mypy type hint checking into pre-commit #9108

Merged
merged 4 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ jobs:
default_python: "3.12"
envs: |
- linux: check-dependencies
- linux: check-types
latest_crds_contexts:
uses: ./.github/workflows/contexts.yml
crds_context:
Expand Down
4 changes: 4 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ repos:
- id: ruff
args: ["--fix"]
- id: ruff-format
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v1.6.1 # Use the latest stable version
hooks:
- id: mypy
- repo: https://github.com/numpy/numpydoc
rev: v1.8.0
hooks:
Expand Down
8 changes: 2 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,14 +427,10 @@ The following three style checks are performed:
required for contributions. If type hints are used, though, their compliance with
[PEP-484](https://peps.python.org/pep-0484/) standards
is enforced using [mypy](https://mypy.readthedocs.io/en/stable/index.html).
To run these checks locally, first `pip install mypy` then use the command
To run these checks locally, use the command

>> mypy .
>> pre-commit run mypy

from within the `jwst` repository. This check is not run through pre-commit;
it's instead handled separately from a different CI workflow. This means that if you are
using type hints, you may encounter a situation where `pre-commit` passes locally but
the CI checks fail on a pull request.

---
**Note:** At time of writing, many submodules in the repository do not yet conform to the style rules;
Expand Down
2 changes: 1 addition & 1 deletion jwst/badpix_selfcal/badpix_selfcal.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def badpix_selfcal(minimg: np.ndarray,
flagfrac_lower: float = 0.001,
flagfrac_upper: float = 0.001,
kernel_size: int = 15,
dispaxis=None) -> tuple:
dispaxis=None) -> np.ndarray:
"""
Flag residual artifacts as bad pixels in the DQ array of a JWST exposure

Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ warn_return_any = true
warn_unused_configs = true
disable_error_code = "import-untyped" # do not check imports
exclude = ["build"]
python_version = "3.10"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does mypy default to something <3.10 without this line? We'll have to keep this line in mind wrt. the upcoming drop of 3.10 support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is not documented, but I figure that (similar to what we decided with the other style checks) this should equal the minimum supported version. I agree it will need to be changed when 3.10 support is dropped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tapastro is that ok with you? if so I will go ahead and merge

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's fine. Merge away 🫡


[[tool.mypy.overrides]]
# don't complain about the installed c parts of this library
Expand Down
8 changes: 0 additions & 8 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@ envlist =
# tox -l -v
#

[testenv:check-types]
description = check type hints, e.g. with mypy
deps =
mypy
types-requests
commands =
mypy jwst --config-file pyproject.toml

[testenv:check-dependencies]
description = verify that install_requires in setup.cfg has correct dependencies
# `extras` needs to be empty to check modules without additional dependencies
Expand Down
Loading