Skip to content

slope: widen name type hint to Optional[str] for terrain-family parity#2687

Merged
brendancol merged 4 commits into
mainfrom
deep-sweep-api-consistency-slope-2026-05-29
May 30, 2026
Merged

slope: widen name type hint to Optional[str] for terrain-family parity#2687
brendancol merged 4 commits into
mainfrom
deep-sweep-api-consistency-slope-2026-05-29

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #2681

slope() annotated its name parameter as str, but aspect(), curvature(), northness(), and eastness() all use Optional[str]. Since name flows into xr.DataArray(name=name) (which accepts None), slope(agg, name=None) already worked at runtime; the annotation was just wrong.

  • Widen name: str = 'slope' to name: Optional[str] = 'slope' and import Optional.
  • Add a test asserting the name annotation matches the rest of the terrain family.
  • Add a test that slope(agg, name=None) returns a DataArray with name is None.

Widening a type hint is non-breaking, so no deprecation shim is needed.

Backend coverage: type-hint-only change on the public entry point; no backend logic touched. numpy / cupy / dask+numpy / dask+cupy paths are unchanged.

Test plan:

  • pytest xrspatial/tests/test_slope.py (43 passed locally)
  • new parity + name=None tests pass

@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 29, 2026

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Domain-aware review (api-consistency sweep, slope module).

Scope: widen slope()'s name annotation from str to Optional[str] so it matches aspect, curvature, northness, and eastness, plus two tests.

Blockers: none.

Suggestions: none. The change is the minimal correct fix. name flows straight into xr.DataArray(name=name), which accepts None, so widening the hint matches runtime behavior and the rest of the terrain family. No deprecation shim is needed since widening a type hint is non-breaking.

Nits: none. Optional import is placed correctly alongside the existing Union import; both are now used.

Verification:

  • get_type_hints(slope)['name'] resolves to str | None, matching aspect.
  • slope(agg, name=None) returns a DataArray with name is None.
  • Full test_slope.py passes (43 tests). No backend logic touched, so numpy/cupy/dask paths are unaffected.

@brendancol brendancol merged commit a6fdc14 into main May 30, 2026
7 checks passed
Melissari1997 pushed a commit to Melissari1997/xarray-spatial that referenced this pull request May 31, 2026
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.

slope() annotates name as str while aspect/curvature use Optional[str]

1 participant