diff --git a/.claude/sweep-api-consistency-state.csv b/.claude/sweep-api-consistency-state.csv index 7d90da5c4..c6cd57986 100644 --- a/.claude/sweep-api-consistency-state.csv +++ b/.claude/sweep-api-consistency-state.csv @@ -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-, asarray-) 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-) - 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." diff --git a/xrspatial/surface_distance.py b/xrspatial/surface_distance.py index c91aeb13e..b817070b3 100644 --- a/xrspatial/surface_distance.py +++ b/xrspatial/surface_distance.py @@ -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') @@ -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', @@ -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. @@ -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', @@ -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', diff --git a/xrspatial/tests/test_surface_distance.py b/xrspatial/tests/test_surface_distance.py index 74ae62fbd..353ed3c4b 100644 --- a/xrspatial/tests/test_surface_distance.py +++ b/xrspatial/tests/test_surface_distance.py @@ -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 # ---------------------------------------------------------------------------