Skip to content

Commit 33d252d

Browse files
authored
Merge pull request #24 from RolnickLab/add-mypy
Add mypy
2 parents 70fc5d3 + 76623c7 commit 33d252d

31 files changed

Lines changed: 824 additions & 230 deletions

.github/workflows/lint.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,4 @@ jobs:
3939
4040
- name: Run linting checks
4141
run: |
42-
make check-pylint
42+
make pylint

.make/lint.make

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ precommit: ## Run Pre-commit on all files manually (Only lint target that works
88
check-lint: ## Check code linting (black, isort, flake8, docformatter and pylint)
99
@$(ENV_COMMAND_TOOL) nox -s check
1010

11-
.PHONY: check-pylint
12-
check-pylint: ## Check code with pylint
13-
@$(ENV_COMMAND_TOOL) nox -s pylint
14-
1511
.PHONY: check-complexity
1612
check-complexity: ## Check code cyclomatic complexity with Flake8-McCabe
1713
@$(ENV_COMMAND_TOOL) nox -s complexity
@@ -20,6 +16,18 @@ check-complexity: ## Check code cyclomatic complexity with Flake8-McCabe
2016
fix-lint: ## Fix code linting (autoflake, autopep8, black, isort, flynt, docformatter)
2117
@$(ENV_COMMAND_TOOL) nox -s fix
2218

19+
.PHONY: autotyping
20+
autotyping: ## Add basic types using 'autotyping'
21+
@$(ENV_COMMAND_TOOL) nox -s autotyping
22+
23+
.PHONY: mypy
24+
mypy: ## Check code with mypy
25+
@$(ENV_COMMAND_TOOL) nox -s mypy
26+
27+
.PHONY: pylint
28+
pylint: ## Check code with pylint
29+
@$(ENV_COMMAND_TOOL) nox -s pylint
30+
2331
.PHONY: markdown-lint
2432
markdown-lint: ## Fix markdown linting using mdformat
2533
@$(ENV_COMMAND_TOOL) nox -s mdformat

.make/scripts/auto_init_script.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -612,7 +612,7 @@ def self_update_for_next_run_of_script(
612612
project_name: str | Any,
613613
python_version: str | Any,
614614
repo_url: str | None,
615-
):
615+
) -> None:
616616
self_replacements = {
617617
"PLACEHOLDER_PACKAGE_NAME": package_name,
618618
"PLACEHOLDER_IMPORT_NAME": package_name,
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# 🎯 Objective & Context
2+
3+
The objective is to resolve 51 static type checking errors flagged by `mypy` across the `geospatial_tools` codebase. These errors primarily consist of implicit `Optional` types, incorrect path handling (mixing `str` and `pathlib.Path`), invariant list types (e.g., `list[Path]` instead of `Sequence[Path]`), and missing type annotations for dictionaries/lists. Addressing these issues will enforce strict typing, improving the reliability and maintainability of the project.
4+
5+
# 🏗️ Architectural Approach
6+
7+
The solution will strictly adhere to modern Python typing standards as outlined in the project's Python skill instructions:
8+
9+
- **Explicit Optionals:** Convert implicit `= None` defaults to explicit `T | None`.
10+
- **Path Handling:** Ensure consistent use of `pathlib.Path` for all file system operations. Where functions accept both `str` and `Path`, standardize types or ensure safe casting.
11+
- **Covariant Sequences:** Replace `list[T]` with `Sequence[T]` (from `typing` or `collections.abc`) in function arguments where variance causes type checking failures.
12+
- **Precise Annotations:** Add exact type annotations for class attributes and local variables that Mypy cannot infer.
13+
14+
This approach aligns with the principle of "Easier To Change" by clearly documenting interfaces through types without altering runtime behavior.
15+
16+
# 🧪 Verification & Failure Modes
17+
18+
- **Verification:** The primary verification method is running `make mypy`. The task is complete when `make mypy` exits with code 0 (no errors found). Additionally, `make test`, `make precommit` and `make pylint` must pass to ensure type changes did not introduce runtime regressions.
19+
- **Failure Modes:**
20+
- Overly broad type annotations (`Any`) masking true issues.
21+
- Incorrectly handling `None` checks, leading to runtime `AttributeError`s.
22+
- Breaking external scripts that depend on functions previously accepting less strict types. This will be mitigated by ensuring changes are backward-compatible (e.g., keeping union types where necessary).
23+
24+
# 📋 Implementation Steps
25+
26+
1. **Fix Implicit Optionals:** Update all function signatures across `raster.py`, `vector.py`, and `nimrod.py` to explicitly type parameters defaulting to `None` as `T | None`.
27+
2. **Resolve Path vs String Mismatches:** Update variable types and division operators in `resample_tiff_raster.py`, `product_search.py`, `download_and_process.py`, and `planetary_computer/sentinel_2.py` to properly handle `pathlib.Path`.
28+
3. **Fix Sequence and List Variance:** Update function arguments in `stac.py`, `utils.py`, and `planetary_computer/sentinel_2.py` to use `Sequence` instead of `list` where covariant types are expected.
29+
4. **Add Missing Annotations and Fix Dictionary/List Initialization:** Add explicit types to class variables in `planetary_computer/sentinel_2.py` and local variables in `utils.py` and `raster.py`.
30+
5. **Address Specific Edge Cases:** Fix the `logging_level` type assignment in `utils.py`, the `zip` argument in `raster.py`, the return statement in `planetary_computer/sentinel_2.py`, and the `sortby` parameter type in `stac.py`.
31+
6. **Fix Missed Errors:** Address type and annotation issues in `download.py`, `vector.py` (lines 113, 141, 338), and `test_copernicus.py` (line 144) not covered by initial tasks.
32+
33+
# 🤝 Next Step
34+
35+
Do you approve Step 1 of the implementation plan to fix the implicit optionals?
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Task: Fix Implicit Optionals
2+
3+
## Goal
4+
5+
Resolve all "implicit Optional" type errors (PEP 484) by explicitly typing parameters defaulting to `None` as `T | None`.
6+
7+
## Context & References
8+
9+
- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
10+
- **Error Pattern:** `Incompatible default for parameter ... (default has type "None", parameter has type "T")`
11+
- **Target Files:**
12+
- `src/geospatial_tools/raster.py` (Lines 248, 249)
13+
- `src/geospatial_tools/vector.py` (Lines 84, 123, 124, 269)
14+
- `src/geospatial_tools/radar/nimrod.py` (Line 20)
15+
16+
## Subtasks
17+
18+
1. **Modify `raster.py`:** Update `merge_raster_bands` parameters `merged_band_names` and `merged_metadata` to use `| None`.
19+
2. **Modify `vector.py`:** Update `create_grid_from_bbox`, `create_grid_from_polygon`, and `save_grid_to_file` parameters (`crs`, `num_of_workers`) to use `| None`.
20+
3. **Modify `nimrod.py`:** Update `extract_nimrod_from_archive` parameter `output_directory` to use `| None`.
21+
22+
## Requirements & Constraints
23+
24+
- Use modern Python union syntax (`T | None`) instead of `Optional[T]`.
25+
- Ensure `None` checks are properly handled in the function bodies if they aren't already.
26+
27+
## Acceptance Criteria (AC)
28+
29+
- [x] `make mypy` no longer reports "implicit Optional" errors for these files.
30+
- [x] Code remains functional and passes existing tests.
31+
32+
## Testing & Validation
33+
34+
- Run `make mypy` to verify fix.
35+
- Run `make test` to ensure no regressions.
36+
37+
## Completion Protocol
38+
39+
- [x] Verify ACs.
40+
- [x] `git add` changed files and `git commit -m "refactor(typing): fix implicit optionals in raster, vector, and nimrod"`
41+
- [x] Update this document with completion status.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Task: Resolve Path vs String Mismatches
2+
3+
## Goal
4+
5+
Ensure consistent use of `pathlib.Path` for file system operations and resolve operator errors (e.g., `/` on strings).
6+
7+
## Context & References
8+
9+
- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
10+
- **Error Pattern:** `Incompatible types in assignment`, `Unsupported left operand type for / ("str")`, `Argument X to "Y" has incompatible type "str"; expected "Path"`
11+
- **Target Files:**
12+
- `scripts/resample_tiff_raster.py` (Lines 44, 45, 46, 48, 68)
13+
- `src/geospatial_tools/planetary_computer/sentinel_2.py` (Lines 398, 406, 414)
14+
- `scripts/sentinel_2_search_and_process/product_search.py` (Lines 156, 157)
15+
- `scripts/sentinel_2_search_and_process/download_and_process.py` (Lines 56, 68, 69)
16+
17+
## Subtasks
18+
19+
1. **Fix `resample_tiff_raster.py`:** Update variable type hints and ensure `Path` objects are used for division and function calls.
20+
2. **Fix `planetary_computer/sentinel_2.py`:** Ensure `output_dir` is treated as a `Path` before using the `/` operator.
21+
3. **Fix `product_search.py` & `download_and_process.py`:** Update default arguments and parameter types to accept/use `Path` where appropriate.
22+
23+
## Requirements & Constraints
24+
25+
- Strictly use `pathlib.Path`.
26+
- Use `Path.joinpath()` or the `/` operator only on `Path` objects.
27+
28+
## Acceptance Criteria (AC)
29+
30+
- [x] `make mypy` no longer reports path-related errors in these files.
31+
- [x] Scripts execute successfully without `TypeError` or `AttributeError`.
32+
33+
## Testing & Validation
34+
35+
- Run `make mypy` to verify fix.
36+
- Run relevant scripts or integration tests (if available) to confirm file path handling works.
37+
38+
## Completion Protocol
39+
40+
- [x] Verify ACs.
41+
- [x] `git add` changed files and `git commit -m "refactor(typing): resolve path vs string mismatches"`
42+
- [x] Update this document with completion status.
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# Task: Fix Sequence and List Variance
2+
3+
## Goal
4+
5+
Replace `list[T]` with `Sequence[T]` in function arguments to allow covariant types and resolve variance-related errors.
6+
7+
## Context & References
8+
9+
- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
10+
- **Error Pattern:** `"list" is invariant ... Consider using "Sequence" instead, which is covariant`
11+
- **Target Files:**
12+
- `src/geospatial_tools/stac.py` (Lines 272, 360, 373)
13+
- `src/geospatial_tools/utils.py` (Line 236)
14+
- `src/geospatial_tools/planetary_computer/sentinel_2.py` (Line 238)
15+
- `tests/test_copernicus.py` (Line 164)
16+
17+
## Subtasks
18+
19+
1. **Modify `stac.py`:** Update function signatures (`merge_raster_bands`, etc.) to accept `Sequence[Path | str]` instead of `list`.
20+
2. **Modify `utils.py`:** Update return type or internal typing of `unzip_file` / `extract_nimrod_from_archive` (or related) to use `Sequence`.
21+
3. **Modify `planetary_computer/sentinel_2.py`:** Update `date_ranges` parameter to use `Sequence`.
22+
4. **Modify `test_copernicus.py`:** Ensure test data types match expected sequences.
23+
24+
## Requirements & Constraints
25+
26+
- Import `Sequence` from `collections.abc` (Python 3.9+).
27+
- Do not use `Sequence` for return types unless the function truly returns an immutable sequence; prefer `list` for returns if caller expects mutability, but handle the variance in the *argument* of the receiving function.
28+
29+
## Acceptance Criteria (AC)
30+
31+
- [x] `make mypy` no longer reports variance errors for these files.
32+
- [x] Code functionality remains unchanged.
33+
34+
## Testing & Validation
35+
36+
- Run `make mypy`.
37+
- Run `make test`.
38+
39+
## Completion Protocol
40+
41+
- [x] Verify ACs.
42+
- [x] `git add` changed files and `git commit -m "refactor(typing): fix sequence and list variance issues"`
43+
- [x] Update this document with completion status.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Task: Address Missing Annotations and Edge Cases
2+
3+
## Goal
4+
5+
Provide missing type annotations for dictionaries/lists and fix specific logic-related type errors.
6+
7+
## Context & References
8+
9+
- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
10+
- **Error Patterns:** `Need type annotation for "params"`, `Missing return statement`, `Incompatible return value type`, `Unsupported right operand type for in`.
11+
- **Target Files:**
12+
- `src/geospatial_tools/utils.py` (Lines 49, 96, 165, 170)
13+
- `src/geospatial_tools/raster.py` (Lines 178, 181, 312)
14+
- `src/geospatial_tools/planetary_computer/sentinel_2.py` (Lines 69, 70, 71, 213, 301, 308)
15+
- `src/geospatial_tools/stac.py` (Lines 234, 657)
16+
17+
## Subtasks
18+
19+
1. **Fix `utils.py`:** Add annotation for `params`, fix `logging_level` assignment, and handle `dataset_crs` type checks properly.
20+
2. **Fix `raster.py`:** Fix `gdf` indexing (ensure it's a GeoDataFrame), fix `zip` argument type, and correct return type of `merge_raster_bands`.
21+
3. **Fix `planetary_computer/sentinel_2.py`:** Add annotations for result attributes, add missing return statement in `sentinel_2_complete_tile_search`, and fix unpacking error in `future.result()`.
22+
4. **Fix `stac.py`:** Handle `self.bands` being `None` in membership checks and fix `sortby` parameter type.
23+
24+
## Requirements & Constraints
25+
26+
- Avoid `Any` where possible; use specific types or `TypeVar` if needed.
27+
- Ensure `future.result()` unpacking matches the actual return type of the submitted function.
28+
29+
## Acceptance Criteria (AC)
30+
31+
- [x] `make mypy` passes with 0 errors.
32+
- [x] All logical fixes are verified by tests.
33+
34+
## Testing & Validation
35+
36+
- Run `make mypy`.
37+
- Run `make test`.
38+
- Run `make precommit`.
39+
40+
## Completion Protocol
41+
42+
- [x] Verify ACs.
43+
- [x] `git add` changed files and `git commit -m "refactor(typing): fix missing annotations and specific edge cases"`
44+
- [x] Update this document with completion status.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Task: Address Missed Mypy Errors
2+
3+
## Goal
4+
5+
Resolve the remaining mypy errors that were not covered by Tasks 1-4, ensuring 100% type checking coverage.
6+
7+
## Context & References
8+
9+
- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
10+
- **Target Files:**
11+
- `src/geospatial_tools/download.py` (Line 32)
12+
- `src/geospatial_tools/vector.py` (Lines 113, 141, 338)
13+
- `tests/test_copernicus.py` (Line 144)
14+
15+
## Subtasks
16+
17+
1. **Fix `download.py`:** Update type checking or argument for `unzip_file` (handle `Path | None`).
18+
2. **Fix `vector.py`:** Add correct type annotations for `properties` dict, narrow type of `bounding_box` in `create_grid_from_bbox`, and fix return type for `save_grid_to_file`.
19+
3. **Fix `test_copernicus.py`:** Update `bbox` argument to use a tuple `(float, float, float, float)`.
20+
21+
## Requirements & Constraints
22+
23+
- Follow modern typing conventions (`T | None`, `pathlib.Path`, etc.).
24+
- Ensure no runtime regressions.
25+
26+
## Acceptance Criteria (AC)
27+
28+
- [x] `make mypy` passes with 0 errors.
29+
30+
## Testing & Validation
31+
32+
- Run `make mypy`.
33+
- Run `make test`.
34+
35+
## Completion Protocol
36+
37+
- [x] Verify ACs.
38+
- [x] `git add` changed files and `git commit -m "refactor(typing): fix missed mypy errors across various modules"`
39+
- [x] Update this document with completion status.

noxfile.py

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ def check(session):
7171
session.run("black", "--check", *paths["all"], external=True)
7272
session.run("isort", *paths["all"], "--check", external=True)
7373
session.run("flynt", *paths["all"], external=True)
74-
# session.run("mypy", *paths["root"], external=True)
74+
session.run("mypy", *paths["root"], external=True)
7575
session.run(
7676
"docformatter",
7777
"--config",
@@ -137,26 +137,16 @@ def flynt(session):
137137
session.run("flynt", *paths["all"], external=True)
138138

139139

140-
# @nox.session()
141-
# def mypy(session):
142-
# paths = get_paths(session)
143-
# session.run("mypy", *paths["root"], external=True)
144-
145-
146-
# @nox.session(name="mypy-install-types")
147-
# def mypy_install_types(session):
148-
# paths = get_paths(session)
149-
# session.run("mypy", "--install-types", "--non-interactive", *paths["root"], external=True)
140+
@nox.session()
141+
def mypy(session):
142+
paths = get_paths(session)
143+
(session.run("mypy", *paths["root"], external=True))
150144

151145

152-
# @nox.session(name="mypy-fix")
153-
# def mypy_fix(session):
154-
# paths = get_paths(session)
155-
# # Generate report
156-
# with open("mypy_report.txt", "w", encoding="utf-8") as f:
157-
# session.run("mypy", *paths["root"], stdout=f, external=True, success_codes=[0, 1])
158-
# # Run upgrade
159-
# session.run("mypy-upgrade", "mypy_report.txt", external=True)
146+
@nox.session()
147+
def autotyping(session):
148+
paths = get_paths(session)
149+
session.run("autotyping", "--aggressive", *paths["all"], external=True)
160150

161151

162152
@nox.session()

0 commit comments

Comments
 (0)