Skip to content

Commit 3837bba

Browse files
authored
Pathfinding API consistency: preserve attrs in multi_stop_search, Dataset support for a_star_search, type hints, defaults (#3653)
* Fix pathfinding API consistency: attrs merge, Dataset support, type hints, defaults (#3644) - multi_stop_search now preserves input surface attrs alongside waypoint_order/segment_costs/total_cost - a_star_search gets @supports_dataset like its sibling and the rest of the library; ds.xrs.a_star_search added to the Dataset accessor - start/goal annotated np.ndarray (was np.array, a function), x/y annotated plain str (None never worked), friction Optional - barriers mutable [] default replaced with None sentinel in both public functions, behavior unchanged - docstring fixes: surface description ("values to bin" leftover), connectivity description, numpy-style spacing Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4 * Address review: bind supports_dataset first param by keyword too (#3644) The wrapper only accepted the raster positionally, so decorating a_star_search regressed a_star_search(surface=...) calls (and cost_distance(raster=...) etc. were already broken the same way on main). The wrapper now resolves the first parameter by its real name when passed as a keyword. Also mention Dataset in the a_star_search Returns section and note why the barriers-default test is numpy-only. Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4
1 parent df8b304 commit 3837bba

7 files changed

Lines changed: 158 additions & 26 deletions

File tree

.claude/sweep-api-consistency-state.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ interpolate,2026-06-12,3285,MEDIUM,2,"Sweep 2026-06-12 (deep-sweep-api-consisten
88
mcda,2026-06-10,3148,HIGH,1;2;3;5,"Sweep 2026-06-10 (deep-sweep-api-consistency-mcda-2026-06-10). Fixed in this branch (#3148): (HIGH Cat 1) owa() named its criterion-weight dict criterion_weights while wlc/wpm/sensitivity use weights (same semantics, same _validate_weights); renamed to weights with keyword-only criterion_weights deprecation shim (DeprecationWarning; both names -> TypeError; positional callers untouched). (MEDIUM Cat 2) boolean_overlay annotated criteria as dict-only while every sibling combiner takes xr.Dataset; Dataset already worked via the Mapping interface -- now annotated/documented as xr.Dataset | dict. (MEDIUM Cat 3) ahp_weights docstring Raises claimed ValueError on incomplete comparisons but code warns (UserWarning) and defaults missing pairs to 1 -- docstring now documents Warns behaviour. (MEDIUM Cat 5) ConsistencyResult returned by public ahp_weights but absent from xrspatial/mcda __all__ and docs/source/reference/mcda.rst -- exported and documented. Documented, NOT fixed here: (MEDIUM Cat 2, deferred to parallel sweep-metadata sibling to avoid duplicate PR) constrain() drops attrs via xr.where while the other nine public functions preserve them. (LOW Cat 2) ahp_weights returns (weights, ConsistencyResult) tuple vs rank_weights bare dict -- intentional, documented in both docstrings, no fix. (LOW Cat 4) name=None inherit-input-name (standardize/constrain) vs literal-name defaults (combiners) -- defensible split, document only. Pre-existing backend bugs surfaced by the mandated cupy smoke (accuracy/test-coverage lane, recorded in #3148 body): owa fails on cupy (numpy order-weights array mixed into cupy multiply, combine.py ~336-340) and on ANY dask backend at graph construction (da.sort does not exist, combine.py:356, despite the owa MemoryError message recommending dask); sensitivity(method=monte_carlo) fails on cupy (template.values implicit-conversion guard). constrain on cupy blocked by the known library-wide cupy 13.6 + xarray xr.where astype incompat (dependency-pin issue), not mcda-specific. cuda-validated: CUDA_AVAILABLE=True; all 10 public functions smoke-tested on cupy DataArrays; owa weights=/criterion_weights= shim verified on numpy AND cupy entry points (cupy execution stops at the pre-existing mixed-array bug, signature acceptance confirmed)."
99
morphology,2026-06-20,3399,MEDIUM,5,"Sweep 2026-06-20 (deep-sweep-api-consistency-morphology-2026-06-20). 1 MEDIUM Cat 5 finding filed as #3399, fixed in this branch (PR #3409). Cat 5 accessor-parity gap: all 7 public morphology functions are exported in xrspatial/__init__.py, documented, and tested, but the .xrs accessor exposed only morph_erode/dilate/opening/closing on both XrsSpatialDataArrayAccessor and XrsSpatialDatasetAccessor; morph_gradient/white_tophat/black_tophat were missing (da.xrs.morph_gradient -> AttributeError while da.xrs.morph_erode works). Root cause: base 4 ops landed #949 + accessors #1042; derived 3 ops landed later #1026 and the accessor was never updated. Fix adds the 3 forwarding methods to both accessor classes (mechanical, matches existing pattern) plus accessor tests (none existed for morph before) and guards all 7 method names in the expected-methods checks. Clean elsewhere: Cat 1 no in-module naming drift -- all 7 publics share the exact signature (agg, kernel=None, boundary='nan', name='<op>'), verified programmatically; matches kernel-op siblings convolution_2d/focal.apply/hotspots which also use agg/kernel/boundary/name. Cat 2 no return drift (all 7 return xr.DataArray with coords/dims/attrs preserved via _dispatch / @supports_dataset). Cat 3 docstring/signature parity exact on all 7 (every param documented, Returns sections all DataArray). Cat 4 no default drift (kernel=None->3x3 ones, boundary='nan' uniform across all 7). Cross-cutting, notes only per template: convolution_2d orders name before boundary while morphology orders boundary before name (both keyword-defaulted, cosmetic); focal.apply/hotspots accept a keyword-only raster= alias for agg but that alias is focal-only (not a library convention) so morphology lacking it is not drift; library-wide first-arg agg vs raster drift spans 20+ modules, out of per-module scope. cuda-validated: CUDA_AVAILABLE=True on this host; all 7 publics smoke-tested with identical kwargs on numpy AND cupy DataArrays (shape parity, no signature drift between numpy/cupy entry points). PR reviewed (COMMENTED), no findings; branch merged with origin/main (clean), left BLOCKED on REVIEW_REQUIRED for user merge."
1010
multispectral,2026-06-20,3433,MEDIUM,3,"Sweep 2026-06-20 (deep-sweep-api-consistency-multispectral). 18 public funcs, all single 2D DataArray returns except true_color (3D composite, inherently different). (#3433 MEDIUM Cat 3) nbr() docstring documented swir_agg but signature param is swir2_agg; copying the name from docs raises TypeError. Docs-only rename to swir2_agg + guard test test_docstring_params_match_signature over the 17 index funcs. No deprecation needed. LOW, documented not fixed: (Cat 5) gci/nbr2/ndmi/true_color/ebbi are NOT re-exported in xrspatial/__init__.py while 13 module siblings are; not an orphan API since the canonical documented path is xrspatial.multispectral.X (all 18 in multispectral.rst) and tests import from the submodule -- convenience-surface gap only. (Cat 3 LOW) only savi types name: str; true_color lacks band type hints and uses r/g/b (conventional for an RGB composite). Cross-module note (not filed per template): multispectral has no -> xr.DataArray return annotations while sibling fire.py annotates all 7 -- library-wide convention drift. No Cat 1 in-module (all band inputs are <band>_agg), no Cat 2 (return shapes consistent), no Cat 4 (no mutable defaults; soil_factor=1.0 consistent evi/savi). CUDA available: numpy+cupy smoke-tested, signatures parity-clean, full suite 171 passed."
11+
pathfinding,2026-07-08,3644,MEDIUM,2;3;4;5,"Sweep 2026-07-08 (deep-sweep-api-consistency-pathfinding-2026-07-08). 4 MEDIUM, all FIXED in one PR (issue #3644): (Cat 2) multi_stop_search replaced input attrs with routing metadata so crs/res/units were lost while sibling a_star_search preserved them -- now merges surface.attrs with waypoint_order/segment_costs/total_cost. (Cat 5) a_star_search lacked @supports_dataset while multi_stop_search and library norm (cost_distance/proximity/surface_distance/hydro) have it; Dataset accessor exposed only multi_stop_search -- added decorator + ds.xrs.a_star_search. (Cat 3) start/goal annotated Union[tuple,list,np.array] (np.array is a function, fixed to np.ndarray); x/y annotated Optional[str] but None crashes (siblings use str, fixed); friction missing Optional; docstring said surface is '2D array of values to bin' (copy-paste) and connectivity had no description -- all corrected. (Cat 4) barriers: list = [] mutable default in both public funcs (never mutated, but siblings use None sentinel) -- now Optional[list]=None with in-body substitution; test asserts None==[]==omitted parity. Repro executed numpy + dask; cupy + dask+cupy smoke-tested on GPU host (Dataset routing, attrs merge, barriers=None all pass). DOCUMENTED not fixed (Cat 1, cross-cutting): first param named surface here vs raster (cost_distance/proximity/surface_distance) vs agg (terrain/focal); library split three ways so a per-module rename shim would not converge anything. LOW noted: a_star_search snap_start/snap_goal vs multi_stop_search single snap kwarg is intentional (per-endpoint vs per-waypoint semantics)."
1112
perlin,2026-06-23,3465,MEDIUM,3,"Sweep 2026-06-23 (deep-sweep-api-consistency-perlin-2026-06-23). 1 MEDIUM Cat 3 finding filed as #3465, fixed in PR #3468. perlin() signature has name: str = 'perlin' (perlin.py:288) but the docstring Parameters section documented only agg/freq/seed, omitting name; sibling generate_terrain() documents its identical name param (terrain.py:607). Fix adds 'name : str, default=""perlin"" / Name for the output DataArray.' matching the terrain style. Pure docstring fix, no signature/behavior change, no deprecation shim needed. Tests: test_perlin_name_param (default+custom name honored), test_perlin_docstring_documents_name (regression). 14/14 test_perlin.py pass incl GPU paths. No HIGH/CRITICAL. Cat 1 clean: agg/seed/name consistent with generate_terrain. Cat 2 clean: all generators return xr.DataArray. Cat 4: seed default 5 (perlin) vs 10 (terrain) is a per-generator domain default, not surprise drift. Cat 5 clean: perlin in __init__.py. cuda-validated: CUDA_AVAILABLE=True on this host, numpy and cupy entry points both accept/propagate name. Cross-cutting, notes only: bump() exposes no name param while perlin/generate_terrain both do -- adding name to bump is a separate bump-module change, out of per-module scope."
1213
polygonize,2026-06-12,3306;3307,MEDIUM,1;3,"Re-sweep 2026-06-12 (deep-sweep-api-consistency-polygonize-2026-06-12); prior pass 2026-05-19 (#2148). 2 MEDIUM findings filed and fixed on branches -01/-02 off this one. (#3306, MEDIUM Cat 3, branch -01) column_name docstring says 'Only used if return_type is geopandas or spatialpandas' but _to_geojson also consumes it as the per-feature property key (verified: properties={'myval': 1}); docs-only fix + test pinning geojson property naming. (#3307, MEDIUM sibling-behavior drift, branch -02) return_type is the only polygonize parameter validated AFTER the computation: invalid value runs the full backend (spy-verified 1 invocation before raise) while sibling contours() validates up front and lists allowed values; fix hoists the check into the top validation block with an allowed-values message (existing test matches on prefix, unaffected). Re-confirmed prior dispositions, still documented-only per cross-module rule: (HIGH Cat 1 cross-module) connectivity (polygonize, matches GDAL/rasterio/skimage) vs neighborhood (sieve.py, zonal.regions) for the identical 4|8 rook/queen concept -- rename shim belongs in sieve/zonal, out of polygonize scope; (LOW Cat 1 cross-cutting) raster (polygonize/sieve/clip_polygon) vs agg (contours/terrain family) first-arg drift, library-wide, not filed per-module. No new Cat 2 (return_type dispatch shapes match docstring Returns section exactly); no Cat 4 (atol/rtol mirror numpy.isclose, connectivity=4 == sieve neighborhood=4); Cat 5 LOW documented-only: module has no __all__ and the non-underscore internals generated_jit + Turn leak via import-star; polygonize re-exported in __init__.py and accessor, no orphan API. Docstring/signature parity otherwise exact (all 10 params documented, all annotated). Open polygonize issues #3292/#3293 checked -- no overlap with these findings. cuda-validated: CUDA_AVAILABLE=True on this host; polygonize smoke-tested with identical full kwargs on numpy, cupy (int + float atol/rtol=0), and dask+cupy; no backend signature drift."
1314
proximity,2026-06-09,3090;3091,HIGH,2;3,"Sweep 2026-06-09 (deep-sweep-api-consistency-proximity-2026-06-09). 1 HIGH Cat 2 finding (#3090): dask+numpy (and unbounded dask+cupy, which converts to it) KDTree path violates the documented lowest-flat-index tie-break in allocation()/direction() whenever the raster has >1 chunk column. _collect_region_targets concatenates targets chunk-major (iy outer, ix inner) so the tree's target order is not global row-major; _kdtree_query_lowest_index then ties to the wrong target. Existing tie-break tests put both targets in the same raster row where chunk order coincides with row-major, so they pass. Repro: 5x5, targets 2@(1,3) and 3@(2,2), chunks (5,3), pixel (2,3) tied at d=1 -> numpy gives 2, dask gives 3. Bounded map_overlap paths are fine (local row-major order is offset-invariant). 1 MEDIUM Cat 3 finding (#3091): all 3 public docstrings claim numpy + dask+numpy support only while cupy/dask+cupy backends exist, are dispatched, and are tested (the tie-break paragraphs in the same docstrings name all 4 backends); direction() opens with a stray copy-pasted slope line ('downward slope direction') plus a doubled 'the the'; allocation example output reads as float64 but the function returns float32; stale '# convert to have same type as of input @raster' comment. Within-module Cat 1/4/5 clean: proximity/allocation/direction share an identical signature (raster, x='x', y='y', target_values=None, max_distance=np.inf, distance_metric='EUCLIDEAN'); consistent with surface_distance siblings (raster/x/y/target_values/max_distance); all 6 public symbols (incl. euclidean/manhattan/great_circle_distance) re-exported in __init__.py, no orphan API. Cross-cutting, documented not filed: sibling distance modules (surface_distance, cost_distance, balanced_allocation) use mutable default target_values: list = [] while proximity uses the None sentinel - the mutable-default fix belongs to those modules; proximity's target_values: list = None hint would be more precise as Optional[list] (LOW, matches library style). cuda-validated: CUDA_AVAILABLE=True on this host; proximity/allocation/direction smoke-tested with identical kwargs on numpy, cupy, dask+numpy, dask+cupy (proximity parity passed; allocation/direction parity failure is finding #3090)."

xrspatial/accessor.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2065,6 +2065,10 @@ def surface_direction(self, elevation, **kwargs):
20652065

20662066
# ---- Pathfinding ----
20672067

2068+
def a_star_search(self, start, goal, **kwargs):
2069+
from .pathfinding import a_star_search
2070+
return a_star_search(self._obj, start, goal, **kwargs)
2071+
20682072
def multi_stop_search(self, waypoints, **kwargs):
20692073
from .pathfinding import multi_stop_search
20702074
return multi_stop_search(self._obj, waypoints, **kwargs)

xrspatial/dataset_support.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,29 @@ def supports_dataset(func):
1717
"""
1818
sig = inspect.signature(func)
1919
has_name_param = 'name' in sig.parameters
20+
first_param = next(iter(sig.parameters))
2021

2122
@functools.wraps(func)
22-
def wrapper(agg, *args, **kwargs):
23+
def wrapper(*args, **kwargs):
24+
# Bind the raster whether it was passed positionally or by its
25+
# real keyword name (e.g. cost_distance(raster=...)); a wrapper
26+
# that only accepts it positionally breaks keyword callers.
27+
if args:
28+
agg, rest = args[0], args[1:]
29+
elif first_param in kwargs:
30+
agg, rest = kwargs.pop(first_param), ()
31+
else:
32+
# Missing entirely: let func raise its own TypeError
33+
return func(*args, **kwargs)
2334
if isinstance(agg, xr.Dataset):
2435
results = {}
2536
for var_name in agg.data_vars:
2637
kw = dict(kwargs)
2738
if has_name_param:
2839
kw['name'] = var_name
29-
results[var_name] = func(agg[var_name], *args, **kw)
40+
results[var_name] = func(agg[var_name], *rest, **kw)
3041
return xr.Dataset(results, attrs=agg.attrs)
31-
return func(agg, *args, **kwargs)
42+
return func(agg, *rest, **kwargs)
3243

3344
return wrapper
3445

0 commit comments

Comments
 (0)