developmentseed / developmentseed/titiler-covjson

Add a multi-checker type-check matrix (mypy + basedpyright blocking; pyrefly + ty informational)

Open
#30 0 comments 0 reactions 1 assignee Claimed by @chuckwondo View on GitHub
chore
Dominant language
Python
Stars
1
Forks
1
Avg merge
2d 1h
Merged PRs (30d)
12

Description

## Summary

Extend the existing classifier-derived `typecheck` CI job so that, on every supported Python (3.11--3.13), the codebase is checked by **multiple** type-checkers rather than mypy alone: `mypy` and `basedpyright` as **blocking** strict gates, plus `pyrefly` and `ty` as **informational** (non-blocking) checks. Bring the existing `src/` + `tests/` to green under all blocking checkers, minimizing suppressions by adding a small local `rasterio` stub.

This is the tooling foundation for the consumer typing-conformance suite (#31), which needs a meaningful multi-checker gate to be worth writing.

## Motivation

The current `typecheck` job runs only `mypy --strict`. mypy is one implementation of the Python type system; `basedpyright`, `pyrefly`, and `ty` infer differently. A typing guarantee that holds under mypy but breaks under pyright is a real, consumer-visible inconsistency we currently cannot see. A second strict checker (basedpyright) as a blocking gate, plus two pre-1.0 checkers as informational signal, catches divergences before they reach downstream users.

## Empirical baseline (measured on the post-#27 tree)

Running `basedpyright` in **strict** mode, uniformly over `src/` + `tests/`, with `conftest` import resolution configured, yields **~141 errors** -- none of which are genuine defects in library logic; they are `Unknown`/partially-unknown types flowing from untyped or weakly-typed dependencies. Approximate breakdown:

| Driver | ~count | Mitigation |
| --- | --- | --- |
| `rasterio.CRS` surface (`CRS`, `from_epsg`, `from_string`, `is_geographic`, `to_authority`, ...) + missing-stub | ~60 | **Local `rasterio` stub** (highest leverage; ~40% of all findings) |
| covjson-pydantic `composite` / `values` (roundtrip tests) | ~28 | Investigate upstream typing; annotate locally |
| `pytest.approx` (returns partially-unknown) | ~7 | Test-only; narrow or ignore |
| `numpy.ma` (`MaskedArray` is weakly typed) | ~5 | Targeted annotate/ignore (no stub fixes numpy's own gap) |
| `ucumvert` (untyped, no published stub) | ~5 | Targeted ignore (already has a mypy `ignore_missing_imports` override) |
| rio-tiler `Info.tags` / `band_metadata` | ~3 | Minor annotate |

Notes that shaped this:

- **No published stub packages exist** for the untyped deps -- `types-rasterio`, `rasterio-stubs`, `types-ucumvert`, etc. are all absent from PyPI. Everything else we depend on already ships `py.typed` (numpy, rio-tiler, covjson-pydantic, lark, pyproj, pydantic) or is covered by `types-shapely` in the dev group.
- A **small hand-written local stub** for just the `rasterio.CRS` surface we use is therefore the single highest-leverage action: `CRS` is one class with a handful of constructors and methods, basedpyright reads `typings/` by default, and it eliminates ~40% of findings across both `src/` and `tests/` with real types rather than suppressions.

## Scope

### 1. `typecheck` dependency group + basedpyright config

- Add an opt-in `typecheck` dependency group containing `basedpyright` (it bundles a Node runtime, so keep it out of the default `uv sync`).
- Add `[tool.basedpyright]`: `include = ["src", "tests"]`, `typeCheckingMode = "strict"`. **Do not** set `pythonVersion` (see "Repo conventions" below).
- Configure `conftest` import resolution so basedpyright resolves `from conftest import ...` the way pytest does (e.g., `extraPaths = ["tests"]` or an `executionEnvironments` entry rooted at `tests`). Without this, basedpyright cannot resolve the typed conftest helpers and emits a large cascade of false `Unknown`s.

### 2. Local `rasterio` partial stub

- Add `typings/rasterio/` (a `.pyi` partial stub) covering the `CRS` surface the code actually uses: the constructors (`from_epsg`, `from_string`, `from_proj4`, `from_authority`, ...) and methods/properties (`is_geographic`, `to_authority`, `to_epsg`, ...) referenced in `helpers.py` / `input.py` / tests.
- Keep it minimal and scoped to what we use; it is a typing aid, not a full vendored stub.

### 3. Resolve the residual findings (uniform strict, no blanket disable)

- After the stub, handle the remaining ~80 with **targeted** measures, not a project-wide `reportUnknown*` disable:
- covjson-pydantic `composite` / `values`: confirm whether this is an upstream typing gap worth reporting; annotate locally where reasonable.
- `numpy.ma`, `ucumvert`, `pytest.approx`: a small, individually-justified set of inline ignores or local annotations.
- The goal is a green `basedpyright` strict gate with a minimal, explained suppression footprint.

### 4. Tighten mypy config

- `[tool.mypy]`: add `warn_unreachable = true` and `enable_error_code = ["redundant-expr", "truthy-bool", "ignore-without-code"]`.

### 5. CI `typecheck` job

Extend the existing matrix job (per Python 3.11--3.13) to:

- `uv sync --locked --group typecheck`
- `uv run mypy` -- **blocking**
- `uv run basedpyright` -- **blocking**
- `uvx pyrefly check src tests` -- **informational** (`continue-on-error: true`)
- `uvx ty check src tests` -- **informational** (`continue-on-error: true`)

`pyrefly` and `ty` are pre-1.0; run them via `uvx` and keep them out of the lock.

## Repo conventions to preserve

- **Matrix is the source of truth for the analyzed Python version.** Do **not** set `python_version` (mypy) or `pythonVersion` (basedpyright); let each matrix entry analyze under its own interpreter, so every supported Python's version-gated branches and resolved stubs are actually exercised. (This matches the existing `[tool.mypy]` convention.)
- **`ruff` `target-version` stays inferred** from `requires-python` (no `[tool.ruff] target-version`).

## Acceptance criteria

- The `typecheck` matrix runs mypy + basedpyright (blocking) and pyrefly + ty (informational) on Python 3.11, 3.12, and 3.13.
- Both blocking checkers are **green** on the existing `src/` + `tests/`; informational checkers may report.
- The `rasterio` local stub is present and eliminates the `rasterio.CRS`-driven findings; the remaining suppressions are few and individually justified (no blanket `reportUnknown*` family disable).
- Default `uv sync` (without `--group typecheck`) still works with no Node toolchain.
- `pytest` (incl. `--doctest-modules`) and `mypy --strict` stay green.

## Out of scope

- The `tests/typing/` consumer conformance suite and negative tests (#31); this issue only establishes the multi-checker gate and brings the existing tree to green under it.

Follow-up to #27 / PR #29 (the 3.11 floor that unblocked this).

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.