Skip to content

rasterize: annotate geometries, columns, merge params (#2250)#2252

Merged
brendancol merged 2 commits into
mainfrom
deep-sweep-api-consistency-rasterize-2026-05-21
May 21, 2026
Merged

rasterize: annotate geometries, columns, merge params (#2250)#2252
brendancol merged 2 commits into
mainfrom
deep-sweep-api-consistency-rasterize-2026-05-21

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2250.

Summary

xrspatial.rasterize.rasterize annotated 13 of 16 public parameters plus the return type. The remaining three -- geometries, columns, merge -- were left bare even though the docstring already declared the intended types. IDE hints, mypy/pyright, and sphinx autodoc see a partial picture when annotations stop mid-signature.

This PR adds annotations matching the docstring:

  • geometries: Any -- the docstring's "GeoDataFrame or iterable of (geometry, value)" spans the geopandas / dask_geopandas / list-of-pairs cases; both geopandas backends are optional deps, so a precise Union would require runtime imports of optional packages in the type expression. Any keeps the public contract honest without pulling those imports up to module level.
  • columns: Optional[Sequence[str]] -- matches "list of str, optional".
  • merge: Union[str, Callable] -- matches "str or callable, default 'last'". The precise Callable[[float, np.ndarray, int], float] signature stays in the docstring body under merge_fn.

No behaviour change. Annotations are additive.

Sweep context

Found by /sweep-api-consistency on 2026-05-21 against the rasterize module. Cat 3 (type hints / docstring), MEDIUM.

Cross-module drifts that the sweep also noted but did NOT file per template rules (cross-cutting drift gets filed against the module pair, not against rasterize alone):

  • clip_polygon(nodata=...) vs rasterize(fill=...) -- same concept, different name.
  • clip_polygon(name: Optional[str] = None) vs rasterize(name: str = 'rasterize') -- default-resolution convention drift.
  • polygonize(column_name=...) vs rasterize(column=...) -- column-selector naming drift.

Test plan

  • pytest xrspatial/tests/test_rasterize_signature_annot_2250.py -- new regression suite pins every public-signature param plus the return annotation. The 2250-specific test names the three offending params directly so a regression message points at the right gap.
  • pytest xrspatial/tests/test_rasterize.py -- 206 passed, 2 skipped (unchanged from main).
  • Smoke-tested rasterize(use_cuda=True, merge=callable, ...) on a CUDA-enabled host.

The signature was annotated on 13 of 16 public parameters plus the
return type. The remaining three -- geometries, columns, merge -- were
left bare even though the docstring already declared the intended
types. IDE hints, mypy, and sphinx autodoc all see a partial picture
when annotations stop mid-signature.

- geometries: Any (the docstring's "GeoDataFrame or iterable of
  (geometry, value)" spans the geopandas / dask_geopandas / list-of-
  pairs cases; both geopandas backends are optional deps, so a
  precise Union would require runtime imports of optional packages
  in the type expression. Any keeps the public contract honest
  without forcing the imports.)
- columns: Optional[Sequence[str]] (matches the docstring's
  "list of str, optional")
- merge: Union[str, Callable] (matches "str or callable, default
  'last'"; the precise Callable[[float, np.ndarray, int], float]
  signature is documented in the body of the docstring under merge_fn)

Regression test pins every public-signature param plus the return
annotation so a future contributor can't silently drop annotations
again. The 2250-specific test names the three offending params
directly so a regression message points at the right gap.

Found by deep-sweep api-consistency on rasterize. Cross-module drifts
the sweep also noted (nodata vs fill, name default convention,
column_name vs column) are out of scope per the sweep template -- they
get filed against the cross-cutting module pair, not against rasterize
alone.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 21, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

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

Review: rasterize annotate geometries, columns, merge (#2250)

Blockers

None.

Suggestions

None.

Nits

  • merge: Union[str, Callable] leaves Callable unparameterized. The docstring already spells out merge_fn(pixel, props, is_first) -> float64, so Callable[[float, np.ndarray, int], float] would match. PR body justifies the looser form; not worth churning.

Notes

  • Diff matches the title: three annotations plus one import line.
  • from __future__ import annotations is on at line 12, so annotations stay lazy strings. No runtime cost, no need to pull optional deps to module top. inspect.signature(rasterize) resolves all 16 params plus the return.
  • geometries: Any is the right call. The true union spans geopandas / dask_geopandas / iterable-of-pairs, two of which are optional deps. A precise Union forces module-top imports or forward-ref strings, both worse for IDE hints.
  • columns: Optional[Sequence[str]] is wider than the docstring's "list of str". That matches actual behaviour: the runtime only iterates, so tuple input already worked.
  • Regression test covers the general invariant and names the three #2250 params explicitly, so a future failure points at the right gap.
  • Sweep state CSV records the cross-module drifts (clip_polygon fill/nodata, name default, polygonize column_name vs column) for later sweeps, per template.

Nothing blocking.

@brendancol brendancol merged commit e9bcc0c into main May 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rasterize: add type annotations to geometries, columns, merge params

1 participant