Skip to content
Open
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
1 change: 1 addition & 0 deletions .claude/sweep-api-consistency-state.csv
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ rasterize,2026-06-09,3089,HIGH,1,"Sweep 2026-06-09 (deep-sweep-api-consistency-r
reproject,2026-06-09,3095;3097,HIGH,1;2;3,"Sweep 2026-06-09 (deep-sweep-api-consistency-reproject-2026-06-09). 2 findings filed and fixed: #3095 -> PR #3125, #3097 -> PR #3134 (branches -01/-02 off this one). (HIGH Cat 2, #3095) merge() raises TypeError ('Implicit conversion to a NumPy array is not allowed') on cupy-backed inputs while sibling reproject() supports numpy/cupy/dask+numpy/dask+cupy; crash site _merge_inmemory info['raster'].values (__init__.py:2572); dask-of-cupy fails the same way at compute via _merge_block_adapter -> _reproject_chunk_numpy/np.asarray. _merge.py has a complete _merge_arrays_cupy that is imported in __init__.py:38 but never called (dead GPU plumbing; the unused import alone is lint issue #3083 from the style sweep). Fix: host round-trip on entry (same pattern as _apply_vertical_shift), GPU result out, docstring documents backend handling. (MEDIUM Cat 3, #3097) _vertical.py Returns docstrings claim 'same type as input/height' but geoid_height(DataArray) returns np.ndarray (verified empirically) and the four conversion wrappers return np.float64/np.ndarray; geoid_height converts scalars to Python float but the wrappers do not (sibling scalar-return drift). Docs-only fix. Documented but NOT fixed: (LOW Cat 1) itrf_transform(src=/tgt=) abbreviations vs source_/target_ elsewhere -- prior 2026-05-29 sweep already weighed this and left it as-is (frames, not CRSes); filed #3099 before noticing the prior disposition, then closed it as not-planned to avoid churn. (LOW Cat 5) module docstring 'Public API' section lists only reproject/merge while __all__ exports 10 names (vertical+itrf funcs invisible in help() header; docs/source/reference/reproject.rst autosummary likewise lists only reproject/merge). Cross-cutting, notes only per template: raster/rasters (reproject) vs agg (terrain family) vs source (geotiff); chunk_size (reproject/merge) vs chunks (open_geotiff); resampling+resolution (reproject/merge/accessor) vs method+target_resolution (resample.py -- resample is the outlier, belongs to a resample-module pass, already in resample row's notes). No Cat 4 default drift (resampling='bilinear'/transform_precision=16/chunk_size=None/bounds_policy='auto'/model='EGM96' consistent across siblings). reproject()/merge() kwarg parity smoke-tested on numpy AND cupy DataArrays (merge cupy crash found exactly there). cuda-validated: CUDA_AVAILABLE=True on this host. CI: all GitHub Actions checks green on both PRs; RTD flapped (pending on #3125, fail on #3134 -- repo-wide backlog, change not docs-rendered); PRs left BLOCKED on REVIEW_REQUIRED for the user to merge."
resample,2026-05-27,2544,MEDIUM,3,"Sweep 2026-05-27 (deep-sweep-api-consistency-resample-2026-05-27). 1 MEDIUM Cat 3 finding fixed in this branch (#2544): resample() was the only public symbol in xrspatial.resample without type annotations on any parameter or return; siblings slope/aspect/hillshade/curvature all annotate `agg: xr.DataArray` and `-> xr.DataArray`. Fix adds annotations matching the docstring (agg: xr.DataArray; scale_factor / target_resolution: float | tuple[float, float] | None; method: str; nodata: float | None; name: str) and a `-> xr.DataArray` return type, plus a docstring note that the @supports_dataset decorator accepts Dataset too. Regression test test_resample_signature_annot_2544.py pins every param and the return annotation. Other findings documented but not filed per template: (MEDIUM Cat 1 cross-module) `method` (resample) vs `resampling` (reproject/merge) -- same conceptual parameter, different name, cross-cutting rename, needs design issue. (LOW Cat 1 cross-cutting) first-arg `agg` (resample/slope/aspect/...) vs `raster` (reproject/rasterize/polygonize/sieve) -- library-wide drift, not per-module. (LOW Cat 5) ALL_METHODS imported by tests but not in __all__ (module has no __all__); borderline orphan but used for test parametrisation only. No Cat 2 (returns xr.DataArray as documented). No Cat 4 mutable defaults. resample is exported in xrspatial/__init__.py. cuda-validated: cupy backend smoke-tested with nearest, bilinear, and average on host with CUDA_AVAILABLE=True."
slope,2026-05-29,2681,MEDIUM,3,"Sweep 2026-05-29 (deep-sweep-api-consistency-slope-2026-05-29). 1 MEDIUM Cat 3 finding fixed in this branch (#2681, PR #2687): slope() annotated name as `str` while every terrain-family sibling (aspect/northness/eastness in aspect.py, curvature in curvature.py) uses Optional[str]. name flows into xr.DataArray(name=name) which accepts None, so slope(agg, name=None) already worked at runtime -- the annotation was just wrong and inconsistent. Fix widens to Optional[str] and imports Optional (module previously imported only Union). Non-breaking (type-hint widening), no deprecation shim. Added test_name_annotation_matches_terrain_family (pins parity vs the 4 siblings via get_type_hints, unwrapping @supports_dataset) and test_name_none_accepted (slope(agg, name=None).name is None). Full test_slope.py passes (43). No backend logic touched -- numpy/cupy/dask+numpy/dask+cupy paths unchanged; public signature is shared across backends via ArrayTypeFunctionMapping. Other categories: no Cat 1 in-module rename (slope/aspect share identical public param names agg/name/method/z_unit/boundary); no Cat 2 return drift (returns xr.DataArray/Dataset via @supports_dataset, same coords/dims/attrs convention as siblings); no Cat 4 default drift (name/method='planar'/z_unit='meter'/boundary='nan' match across the family); no Cat 5 orphan API (slope re-exported in __init__.py, documented, no __all__ but consistent with module convention). Cross-cutting (documented, not filed per template): first-arg `agg` (slope/aspect/curvature) vs `raster` (reproject/rasterize/polygonize) is library-wide drift. cuda-validated: CUDA_AVAILABLE=True on this host; cupy slope smoke-tested (planar) and signature parity confirmed between numpy and cupy entry points."
surface_distance,2026-08-16,3712,MEDIUM,1;2;3;4,"Sweep 2026-08-16 (deep-sweep-api-consistency-surface_distance-2026-08-16). 1 MEDIUM fixed, 1 MEDIUM already filed by a sibling sweep, 3 LOW documented. Baseline: pytest xrspatial/tests/test_surface_distance.py 37/37 green; CUDA host, cupy and dask+cupy paths executed for real. Compared the surface_distance/surface_allocation/surface_direction trio against its direct analogues proximity/allocation/direction and cost_distance, plus a 1918-row library-wide ast parameter inventory. FIXED (issue #3712, PR #3718, Cat 1+4 MEDIUM): all three declared target_values: list = [] while proximity/allocation/direction/cost_distance declare None and normalise in the body; target_values=None therefore reached np.asarray(None, dtype=float64) -> 0-d array -> numba TypingError in _seed_sources (getitem(array(float64, 0d, C), int64)), a ~40-line traceback that never names target_values. Same call works on all four siblings. Also removes the mutable default. Non-breaking, no shim needed. Added a 24-case None-vs-[] parity test across numpy/cupy/dask+numpy/dask+cupy x max_distance in (1.5 bounded map_overlap, inf iterative tile) plus a non-vacuous-output assert and a signature guard; 62 pass, up from 37. Review follow-up widened the hint to list | None so it matches the new default (proximity and cost_distance still spell it list = None - cross-cutting, left alone). NOT FILED, duplicate (Cat 2 MEDIUM): dask-backed results leak the internal dask graph key as DataArray.name (xrspatial.surface_distance-<hash>, asarray-<hash>) while numpy/cupy give None and proximity explicitly does result.name = None; observable as to_dataset() raising on numpy but succeeding on dask with a hashed variable name. Already filed as #3708 by the parallel metadata sweep, so no duplicate issue or PR from here. cost_distance leaks the same way (array-<hash>) - cross-cutting, out of this module's scope. LOW, documented only: (a) surface_allocation and surface_direction collapse six parameters onto one docstring line deferring to :func:`surface_distance`, where proximity's allocation and direction document each parameter in full; (b) neither of the three carries the standard 'supports NumPy, CuPy, Dask with NumPy, and Dask with CuPy' paragraph that proximity and cost_distance carry, and method='geodesic' in fact raises NotImplementedError on cupy, dask+numpy and dask+cupy (verified) with no docstring mention; (c) pre-existing flake8 F841 at xrspatial/tests/test_surface_distance.py:234 (unused sd), untouched. NOT FINDINGS (checked and cleared): x/y kwargs work with renamed lon/lat dims because _compute asserts dims == (y, x) before get_dataarray_resolution falls back to dims[-1]/[-2]; surface_direction returns float32 like its siblings (_vectorized_calc_direction casts, so the declared dask dtype=float32 is honest); connectivity=8 and max_distance=np.inf match cost_distance and proximity; method='planar'/'geodesic' matches slope/aspect/curvature, so renaming it to proximity's distance_metric would break the larger convention; all three are re-exported in __init__.py, listed in docs/source/reference/proximity.rst, and mirrored in both accessor classes. Input validation gaps versus proximity (no max_distance sign/NaN check, no target_values finiteness check) left to the parallel error-handling sweep. Cross-cutting: balanced_allocation also declares target_values: list = [] - not fixed here. Also hit while reviewing the dask path, already filed by sibling sweeps so not duplicated: surface_direction returns block-local bearings on the dask iterative route (#3713, #3719); cupy relaxation caps at H+W passes (#3721); Dijkstra heap sized H*W overflows (#3722, #3723); max_distance nan/negative unvalidated (#3711); docstring gaps (#3714)."
templates,2026-06-26,,MEDIUM,3,"Cat3 MEDIUM: from_template/list_templates lacked type hints while sibling generate_terrain is annotated; added hints (non-breaking). resolution param vs res attr naming intentional, not flagged. No __all__ (library norm)."
visibility,2026-06-10,3183,MEDIUM,3;5,"Sweep 2026-06-10 (deep-sweep-api-consistency-visibility-2026-06-10). 2 MEDIUM findings filed as issue #3183, fixed in this branch. (MEDIUM Cat 5) output-name convention drift: viewshed() sets a fixed output name and exposes name=, but cumulative_viewshed (visibility.py:289) and visibility_frequency built/returned DataArrays with name=None. Fix adds name='cumulative_viewshed'/'visibility_frequency' params (Optional[str]) and sets result.name; additive, non-breaking, no shim. coords/attrs were already preserved on both. (MEDIUM Cat 3) line_of_sight (visibility.py:162) annotated frequency_mhz: float = None; default contradicts the float hint and the docstring already says optional. Fix -> Optional[float] (imported typing.Optional). Tests added: cumulative/frequency default+custom name. No Cat 1 naming drift: observer_elev/target_elev/max_distance/x/y and the x0/y0/x1/y1 two-point extension match viewshed and the observers dict keys. No Cat 2 arbitrary return drift: line_of_sight -> Dataset fits its per-sample multi-variable result; the two cumulative funcs -> DataArray like viewshed. No Cat 4 default drift (observer_elev=0/target_elev=0/max_distance=None match). No Cat 5 orphan API: all 3 funcs re-exported in __init__.py; no __all__ but consistent with module convention. cuda-validated: CUDA_AVAILABLE=True on this host; cupy entry points accept the new name= kwarg and the line_of_sight Optional hint. PRE-EXISTING backend bug (out of scope, not an api-consistency issue, NOT filed here): cumulative_viewshed on a cupy raster raises TypeError 'Unsupported type numpy.ndarray' in the count + (vs_data != INVISIBLE) accumulation (numpy accumulator vs cupy viewshed result); reproduced on origin/main without this branch's changes -- a backend-parity gap for a future backend-parity sweep."
zonal,2026-06-10,3188,MEDIUM,1;3;5,"Re-sweep 2026-06-10 (deep-sweep-api-consistency-zonal-2026-06-10). Prior sweep's HIGH zones_ids/zone_ids typo confirmed already fixed on main (#2521). Several previously-documented MEDIUM Cat 3 items also fixed on main since 2026-05-27: crosstab layer docstring now says default=None; hypsometric_integral now has param+return annotations; apply now has -> xr.DataArray. Two remaining safe Cat 3 fixes filed+PR'd this run (issue #3188 / PR #3196): (1) crosstab zone_ids/cat_ids annotated List[...]=None -> wrapped in Optional[...] to match stats()/crop(); (2) crosstab nodata_values docstring said 'Cells with nodata' (copy-paste from apply) -> now references nodata_values. Non-breaking, 17 crosstab tests pass. Documented-not-fixed: (MEDIUM Cat 1) nodata vs nodata_values drift across stats/crosstab (nodata_values, default None, filters VALUES raster) vs apply/hypsometric_integral (nodata, default 0, filters ZONES raster) -- names differ but so do the concepts and defaults, so a blanket rename would conflate two distinct meanings; needs a design decision, not a mechanical shim. (MEDIUM Cat 5) get_full_extent has a public-style docstring+example but is not in __init__.py -- borderline orphan, minor utility, left as-is. (LOW Cat 3) crop() lacks a return type annotation while stats/crosstab/apply/regions/trim annotate theirs. Cross-cutting (not filed): first-arg name varies (stats/crosstab/crop use zones; regions/trim use raster) but regions/trim operate on the raster itself so the name matches the role; library-wide agg vs raster vs values naming spans 20+ modules, out of per-module scope. cuda-validated: CUDA_AVAILABLE=True on this host."
12 changes: 7 additions & 5 deletions xrspatial/surface_distance.py
Original file line number Diff line number Diff line change
Expand Up @@ -1297,6 +1297,8 @@ def _chunk_func(source_block, elev_block):
def _compute(raster, elevation, x, y, target_values, max_distance,
connectivity, method, mode):
"""Core dispatcher for surface_distance / allocation / direction."""
if target_values is None:
target_values = []
_validate_raster(raster, func_name='surface_distance', name='raster')
_validate_raster(elevation, func_name='surface_distance',
name='elevation')
Expand Down Expand Up @@ -1421,7 +1423,7 @@ def surface_distance(
elevation: xr.DataArray,
x: str = "x",
y: str = "y",
target_values: list = [],
target_values: list | None = None,
max_distance: float = np.inf,
connectivity: int = 8,
method: str = 'planar',
Expand All @@ -1445,9 +1447,9 @@ def surface_distance(
Name of the x coordinate.
y : str, default='y'
Name of the y coordinate.
target_values : list, optional
target_values : list, default=None
Specific pixel values in *raster* to treat as sources.
If empty, all non-zero finite pixels are sources.
If ``None`` or empty, all non-zero finite pixels are sources.
max_distance : float, default=np.inf
Maximum surface distance. Pixels beyond this budget are NaN.
A finite value enables efficient Dask parallelisation.
Expand Down Expand Up @@ -1482,7 +1484,7 @@ def surface_allocation(
elevation: xr.DataArray,
x: str = "x",
y: str = "y",
target_values: list = [],
target_values: list | None = None,
max_distance: float = np.inf,
connectivity: int = 8,
method: str = 'planar',
Expand Down Expand Up @@ -1524,7 +1526,7 @@ def surface_direction(
elevation: xr.DataArray,
x: str = "x",
y: str = "y",
target_values: list = [],
target_values: list | None = None,
max_distance: float = np.inf,
connectivity: int = 8,
method: str = 'planar',
Expand Down
41 changes: 41 additions & 0 deletions xrspatial/tests/test_surface_distance.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,47 @@ def test_target_values():
assert sd[0, 1] == pytest.approx(2.0, abs=1e-5)


@pytest.mark.parametrize("backend",
['numpy', 'cupy', 'dask+numpy', 'dask+cupy'])
@pytest.mark.parametrize("func",
[surface_distance, surface_allocation,
surface_direction])
# 1.5 keeps the dask pad below the (3, 3) chunk size so the bounded
# map_overlap route runs; np.inf takes the iterative tile route.
@pytest.mark.parametrize("max_distance", [1.5, np.inf])
def test_target_values_none_matches_empty_list(backend, func, max_distance):
"""target_values=None means the same thing as [] on every backend.

The proximity trio and cost_distance already accept None; issue #3712
reported that the surface_* trio raised a numba TypingError instead.
"""
source = np.zeros((6, 6), dtype=np.float64)
source[0, 0] = 1.0
source[5, 5] = 3.0
elev = np.arange(36, dtype=np.float64).reshape(6, 6) * 0.5

raster = _make_raster(source, backend)
elevation = _make_raster(elev, backend)

with_none = _compute(func(raster, elevation, target_values=None,
max_distance=max_distance))
with_empty = _compute(func(raster, elevation, target_values=[],
max_distance=max_distance))

# An all-NaN comparison would pass vacuously.
assert np.isfinite(with_none).any()
np.testing.assert_allclose(with_none, with_empty, equal_nan=True)


def test_target_values_default_is_not_mutable():
"""The public signatures must not carry a mutable default."""
import inspect

for func in (surface_distance, surface_allocation, surface_direction):
default = inspect.signature(func).parameters['target_values'].default
assert default is None, f"{func.__name__} has a mutable default"


# ---------------------------------------------------------------------------
# Tests — connectivity
# ---------------------------------------------------------------------------
Expand Down
Loading