Skip to content
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
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ jobs:

- name: Run linting checks
run: |
make check-pylint
make pylint
16 changes: 12 additions & 4 deletions .make/lint.make
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ precommit: ## Run Pre-commit on all files manually (Only lint target that works
check-lint: ## Check code linting (black, isort, flake8, docformatter and pylint)
@$(ENV_COMMAND_TOOL) nox -s check

.PHONY: check-pylint
check-pylint: ## Check code with pylint
@$(ENV_COMMAND_TOOL) nox -s pylint

.PHONY: check-complexity
check-complexity: ## Check code cyclomatic complexity with Flake8-McCabe
@$(ENV_COMMAND_TOOL) nox -s complexity
Expand All @@ -20,6 +16,18 @@ check-complexity: ## Check code cyclomatic complexity with Flake8-McCabe
fix-lint: ## Fix code linting (autoflake, autopep8, black, isort, flynt, docformatter)
@$(ENV_COMMAND_TOOL) nox -s fix

.PHONY: autotyping
autotyping: ## Add basic types using 'autotyping'
@$(ENV_COMMAND_TOOL) nox -s autotyping

.PHONY: mypy
mypy: ## Check code with mypy
@$(ENV_COMMAND_TOOL) nox -s mypy

.PHONY: pylint
pylint: ## Check code with pylint
@$(ENV_COMMAND_TOOL) nox -s pylint

.PHONY: markdown-lint
markdown-lint: ## Fix markdown linting using mdformat
@$(ENV_COMMAND_TOOL) nox -s mdformat
Expand Down
2 changes: 1 addition & 1 deletion .make/scripts/auto_init_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ def self_update_for_next_run_of_script(
project_name: str | Any,
python_version: str | Any,
repo_url: str | None,
):
) -> None:
self_replacements = {
"PLACEHOLDER_PACKAGE_NAME": package_name,
"PLACEHOLDER_IMPORT_NAME": package_name,
Expand Down
35 changes: 35 additions & 0 deletions docs/agents/planning/mypy-refactor/mypy-refactor-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# 🎯 Objective & Context

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.

# 🏗️ Architectural Approach

The solution will strictly adhere to modern Python typing standards as outlined in the project's Python skill instructions:

- **Explicit Optionals:** Convert implicit `= None` defaults to explicit `T | None`.
- **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.
- **Covariant Sequences:** Replace `list[T]` with `Sequence[T]` (from `typing` or `collections.abc`) in function arguments where variance causes type checking failures.
- **Precise Annotations:** Add exact type annotations for class attributes and local variables that Mypy cannot infer.

This approach aligns with the principle of "Easier To Change" by clearly documenting interfaces through types without altering runtime behavior.

# 🧪 Verification & Failure Modes

- **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.
- **Failure Modes:**
- Overly broad type annotations (`Any`) masking true issues.
- Incorrectly handling `None` checks, leading to runtime `AttributeError`s.
- 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).

# 📋 Implementation Steps

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`.
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`.
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.
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`.
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`.
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.

# 🤝 Next Step

Do you approve Step 1 of the implementation plan to fix the implicit optionals?
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Task: Fix Implicit Optionals

## Goal

Resolve all "implicit Optional" type errors (PEP 484) by explicitly typing parameters defaulting to `None` as `T | None`.

## Context & References

- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
- **Error Pattern:** `Incompatible default for parameter ... (default has type "None", parameter has type "T")`
- **Target Files:**
- `src/geospatial_tools/raster.py` (Lines 248, 249)
- `src/geospatial_tools/vector.py` (Lines 84, 123, 124, 269)
- `src/geospatial_tools/radar/nimrod.py` (Line 20)

## Subtasks

1. **Modify `raster.py`:** Update `merge_raster_bands` parameters `merged_band_names` and `merged_metadata` to use `| None`.
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`.
3. **Modify `nimrod.py`:** Update `extract_nimrod_from_archive` parameter `output_directory` to use `| None`.

## Requirements & Constraints

- Use modern Python union syntax (`T | None`) instead of `Optional[T]`.
- Ensure `None` checks are properly handled in the function bodies if they aren't already.

## Acceptance Criteria (AC)

- [x] `make mypy` no longer reports "implicit Optional" errors for these files.
- [x] Code remains functional and passes existing tests.

## Testing & Validation

- Run `make mypy` to verify fix.
- Run `make test` to ensure no regressions.

## Completion Protocol

- [x] Verify ACs.
- [x] `git add` changed files and `git commit -m "refactor(typing): fix implicit optionals in raster, vector, and nimrod"`
- [x] Update this document with completion status.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Task: Resolve Path vs String Mismatches

## Goal

Ensure consistent use of `pathlib.Path` for file system operations and resolve operator errors (e.g., `/` on strings).

## Context & References

- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
- **Error Pattern:** `Incompatible types in assignment`, `Unsupported left operand type for / ("str")`, `Argument X to "Y" has incompatible type "str"; expected "Path"`
- **Target Files:**
- `scripts/resample_tiff_raster.py` (Lines 44, 45, 46, 48, 68)
- `src/geospatial_tools/planetary_computer/sentinel_2.py` (Lines 398, 406, 414)
- `scripts/sentinel_2_search_and_process/product_search.py` (Lines 156, 157)
- `scripts/sentinel_2_search_and_process/download_and_process.py` (Lines 56, 68, 69)

## Subtasks

1. **Fix `resample_tiff_raster.py`:** Update variable type hints and ensure `Path` objects are used for division and function calls.
2. **Fix `planetary_computer/sentinel_2.py`:** Ensure `output_dir` is treated as a `Path` before using the `/` operator.
3. **Fix `product_search.py` & `download_and_process.py`:** Update default arguments and parameter types to accept/use `Path` where appropriate.

## Requirements & Constraints

- Strictly use `pathlib.Path`.
- Use `Path.joinpath()` or the `/` operator only on `Path` objects.

## Acceptance Criteria (AC)

- [x] `make mypy` no longer reports path-related errors in these files.
- [x] Scripts execute successfully without `TypeError` or `AttributeError`.

## Testing & Validation

- Run `make mypy` to verify fix.
- Run relevant scripts or integration tests (if available) to confirm file path handling works.

## Completion Protocol

- [x] Verify ACs.
- [x] `git add` changed files and `git commit -m "refactor(typing): resolve path vs string mismatches"`
- [x] Update this document with completion status.
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Task: Fix Sequence and List Variance

## Goal

Replace `list[T]` with `Sequence[T]` in function arguments to allow covariant types and resolve variance-related errors.

## Context & References

- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
- **Error Pattern:** `"list" is invariant ... Consider using "Sequence" instead, which is covariant`
- **Target Files:**
- `src/geospatial_tools/stac.py` (Lines 272, 360, 373)
- `src/geospatial_tools/utils.py` (Line 236)
- `src/geospatial_tools/planetary_computer/sentinel_2.py` (Line 238)
- `tests/test_copernicus.py` (Line 164)

## Subtasks

1. **Modify `stac.py`:** Update function signatures (`merge_raster_bands`, etc.) to accept `Sequence[Path | str]` instead of `list`.
2. **Modify `utils.py`:** Update return type or internal typing of `unzip_file` / `extract_nimrod_from_archive` (or related) to use `Sequence`.
3. **Modify `planetary_computer/sentinel_2.py`:** Update `date_ranges` parameter to use `Sequence`.
4. **Modify `test_copernicus.py`:** Ensure test data types match expected sequences.

## Requirements & Constraints

- Import `Sequence` from `collections.abc` (Python 3.9+).
- 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.

## Acceptance Criteria (AC)

- [x] `make mypy` no longer reports variance errors for these files.
- [x] Code functionality remains unchanged.

## Testing & Validation

- Run `make mypy`.
- Run `make test`.

## Completion Protocol

- [x] Verify ACs.
- [x] `git add` changed files and `git commit -m "refactor(typing): fix sequence and list variance issues"`
- [x] Update this document with completion status.
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Task: Address Missing Annotations and Edge Cases

## Goal

Provide missing type annotations for dictionaries/lists and fix specific logic-related type errors.

## Context & References

- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
- **Error Patterns:** `Need type annotation for "params"`, `Missing return statement`, `Incompatible return value type`, `Unsupported right operand type for in`.
- **Target Files:**
- `src/geospatial_tools/utils.py` (Lines 49, 96, 165, 170)
- `src/geospatial_tools/raster.py` (Lines 178, 181, 312)
- `src/geospatial_tools/planetary_computer/sentinel_2.py` (Lines 69, 70, 71, 213, 301, 308)
- `src/geospatial_tools/stac.py` (Lines 234, 657)

## Subtasks

1. **Fix `utils.py`:** Add annotation for `params`, fix `logging_level` assignment, and handle `dataset_crs` type checks properly.
2. **Fix `raster.py`:** Fix `gdf` indexing (ensure it's a GeoDataFrame), fix `zip` argument type, and correct return type of `merge_raster_bands`.
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()`.
4. **Fix `stac.py`:** Handle `self.bands` being `None` in membership checks and fix `sortby` parameter type.

## Requirements & Constraints

- Avoid `Any` where possible; use specific types or `TypeVar` if needed.
- Ensure `future.result()` unpacking matches the actual return type of the submitted function.

## Acceptance Criteria (AC)

- [x] `make mypy` passes with 0 errors.
- [x] All logical fixes are verified by tests.

## Testing & Validation

- Run `make mypy`.
- Run `make test`.
- Run `make precommit`.

## Completion Protocol

- [x] Verify ACs.
- [x] `git add` changed files and `git commit -m "refactor(typing): fix missing annotations and specific edge cases"`
- [x] Update this document with completion status.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Task: Address Missed Mypy Errors

## Goal

Resolve the remaining mypy errors that were not covered by Tasks 1-4, ensuring 100% type checking coverage.

## Context & References

- **Plan:** `docs/agents/planning/mypy-refactor/mypy-refactor-plan.md`
- **Target Files:**
- `src/geospatial_tools/download.py` (Line 32)
- `src/geospatial_tools/vector.py` (Lines 113, 141, 338)
- `tests/test_copernicus.py` (Line 144)

## Subtasks

1. **Fix `download.py`:** Update type checking or argument for `unzip_file` (handle `Path | None`).
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`.
3. **Fix `test_copernicus.py`:** Update `bbox` argument to use a tuple `(float, float, float, float)`.

## Requirements & Constraints

- Follow modern typing conventions (`T | None`, `pathlib.Path`, etc.).
- Ensure no runtime regressions.

## Acceptance Criteria (AC)

- [x] `make mypy` passes with 0 errors.

## Testing & Validation

- Run `make mypy`.
- Run `make test`.

## Completion Protocol

- [x] Verify ACs.
- [x] `git add` changed files and `git commit -m "refactor(typing): fix missed mypy errors across various modules"`
- [x] Update this document with completion status.
28 changes: 9 additions & 19 deletions noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def check(session):
session.run("black", "--check", *paths["all"], external=True)
session.run("isort", *paths["all"], "--check", external=True)
session.run("flynt", *paths["all"], external=True)
# session.run("mypy", *paths["root"], external=True)
session.run("mypy", *paths["root"], external=True)
session.run(
"docformatter",
"--config",
Expand Down Expand Up @@ -137,26 +137,16 @@ def flynt(session):
session.run("flynt", *paths["all"], external=True)


# @nox.session()
# def mypy(session):
# paths = get_paths(session)
# session.run("mypy", *paths["root"], external=True)


# @nox.session(name="mypy-install-types")
# def mypy_install_types(session):
# paths = get_paths(session)
# session.run("mypy", "--install-types", "--non-interactive", *paths["root"], external=True)
@nox.session()
def mypy(session):
paths = get_paths(session)
(session.run("mypy", *paths["root"], external=True))


# @nox.session(name="mypy-fix")
# def mypy_fix(session):
# paths = get_paths(session)
# # Generate report
# with open("mypy_report.txt", "w", encoding="utf-8") as f:
# session.run("mypy", *paths["root"], stdout=f, external=True, success_codes=[0, 1])
# # Run upgrade
# session.run("mypy-upgrade", "mypy_report.txt", external=True)
@nox.session()
def autotyping(session):
paths = get_paths(session)
session.run("autotyping", "--aggressive", *paths["all"], external=True)


@nox.session()
Expand Down
4 changes: 3 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ dev = [
"pre-commit>=3.7.0",
"flake8-pyproject>=1.2.3",
"docformatter[tomli]>=1.7.5",
"nbval>=0.11.0,<0.12",
"nbval>=0.11.0",
"black[jupyter]>=25.1.0",
"nox>=2024.4.15",
"autoflake>=2.3.1",
Expand All @@ -56,6 +56,8 @@ dev = [
"mdformat-gfm-alerts>=2.0.0",
"pyment>=0.3.3",
"mdformat-mkdocs>=5.1.1",
"mypy>=1.19.1",
"autotyping>=24.9.0",
]
[project.optional-dependencies]
docs =[
Expand Down
4 changes: 3 additions & 1 deletion scripts/resample_tiff_raster.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ def get_source_information(source_image: pathlib.Path):
help="Base output path",
default=PROJECT_DATA_DIR,
)
def resample_tiff(source_image: str, resample_target: str, output_path: str):
def resample_tiff(
source_image: str | pathlib.Path, resample_target: str | pathlib.Path, output_path: str | pathlib.Path
):
source_image = pathlib.Path(source_image)
resample_target = pathlib.Path(resample_target)
output_path = pathlib.Path(output_path)
Expand Down
Loading