From ebc369bb87f9f98ca6f58628b50c71bc734ce87c Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 8 Jul 2026 17:26:48 -0400 Subject: [PATCH 1/2] Validate barriers, search_radius, start/goal, and dims in pathfinding (#3649) String-typed barriers were silently ignored (numba compares float cells to unicode, always False), so a_star_search routed straight through walls the caller asked to block. Negative or float search_radius either returned a silent all-NaN "no path" or crashed deep in slicing code. multi_stop_search with mismatched dim names raised a bare KeyError 'y' where a_star_search raises ValueError, and scalar start/goal points died inside _get_pixel_id. Adds _validate_barriers, _validate_search_radius, _validate_point, and _validate_surface_dims helpers, wires them into both public functions, and improves the dims message to name the actual dims and the x=/y= parameters. Also records the sweep state-CSV row. Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4 --- .claude/sweep-error-handling-state.csv | 1 + xrspatial/pathfinding.py | 81 +++++++++++++++-- xrspatial/tests/test_pathfinding.py | 116 +++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 7 deletions(-) diff --git a/.claude/sweep-error-handling-state.csv b/.claude/sweep-error-handling-state.csv index cfed9433e..38d2bfd25 100644 --- a/.claude/sweep-error-handling-state.csv +++ b/.claude/sweep-error-handling-state.csv @@ -2,3 +2,4 @@ module,last_inspected,issue,severity_max,categories_found,notes bump,2026-07-02,,HIGH,1;2;3;4,"bump() agg template unvalidated: plain ndarray -> ArrayTypeFunctionMapping 'Unsupported Array Type'; 3D/1D DataArray -> 'too many values to unpack'. count/spread unvalidated (internal numpy errors / silent). Added _validate_raster(agg,ndim=2) + _validate_scalar for count,spread. all 4 backends verified (CUDA present)." convolution,2026-07-02,,HIGH,1;2;3;4,"convolve_2d/convolution_2d skipped kernel + DataArray validation: None/1D/3D/list kernel -> numba TypingError, even kernel silently off-center (custom_kernel rejects it), numpy agg -> memoryview astype error. Fixed via _validate_kernel + _validate_raster; branch deep-sweep-error-handling-convolution-2026-07-02 pushed to fork; issue/PR create blocked by auto-mode, open from parent. MEDIUM(unfixed): annulus_kernel inner>outer -> cryptic np.pad 'index cant contain negative values'. LOW: circle_kernel cellsize=0 ZeroDivisionError, cellsize<0 cryptic linspace; calc_cellsize non-DataArray -> AttributeError attrs. cupy verified." geotiff,2026-07-02,3604,MEDIUM,2;4,"to_geotiff 0D/1D DataArray raised opaque IndexError from _coords.py coords_to_transform (dims[-2]) instead of clean 'Expected 2D or 3D' ValueError; numpy path + 4D DataArray already clean. Fixed via early ndim guard before dispatch (eager/vrt/gpu) + 3 tests; PR #3604. Read-side param validation + typed-error hierarchy + allow_rotated/allow_invalid_nodata VRT+chunked opt-in threading verified clean (CUDA available, GPU paths run). gh issue create blocked by auto-mode; PR opened. Cat 2+4." +pathfinding,2026-07-08,3649,CRITICAL,1;2;3;4,"CRITICAL: string barriers (['0']) silently ignored -> path crosses wall, no error/warning (numba float==unicode always False); scalar barriers=0 -> numba TypingError. HIGH: search_radius unvalidated: -1 silently all-NaN 'no path' (numpy+cupy), other geometries 'negative dimensions are not allowed'; float radius -> slice TypeError; via multi_stop -> misleading 'no path between waypoints'. MEDIUM: multi_stop_search missing dims check -> bare KeyError 'y' vs a_star ValueError; scalar start/goal -> 'float object is not subscriptable' in _get_pixel_id. All fixed: _validate_barriers/_validate_search_radius/_validate_point/_validate_surface_dims + tests. LOW (unfixed): (0,0)-size raster -> 'zero-size array to reduction' from calc_res. All 4 backends executed (CUDA present). Battery: numpy/dask/cupy/dask+cupy." diff --git a/xrspatial/pathfinding.py b/xrspatial/pathfinding.py index 3c1a4199c..4a6044c40 100644 --- a/xrspatial/pathfinding.py +++ b/xrspatial/pathfinding.py @@ -30,6 +30,71 @@ _MAX_WAYPOINTS = 1000 +def _validate_surface_dims(surface, x, y, func_name): + """Raise ValueError if *surface* dims do not match the (y, x) names.""" + if surface.dims != (y, x): + raise ValueError( + f"{func_name}(): expected `surface` to have dims ({y!r}, {x!r}), " + f"got {surface.dims}. Pass the actual dimension names via the " + f"`x=` and `y=` parameters." + ) + + +def _validate_barriers(barriers): + """Coerce *barriers* to a 1-D float64 array or raise a clear error. + + Numba compares the (float) cell value against each barrier element, + so a non-numeric dtype would either fail deep inside the kernel with + a TypingError (0-d input) or, worse, compare unequal everywhere and + silently disable all barriers (string input). + """ + arr = np.asarray(barriers) + if arr.ndim != 1: + hint = (f" Did you mean barriers=[{barriers!r}]?" + if arr.ndim == 0 else "") + raise ValueError( + f"barriers must be a 1-D list or array of cell values, " + f"got {arr.ndim}-D input.{hint}" + ) + if arr.size > 0 and not ( + np.issubdtype(arr.dtype, np.integer) + or np.issubdtype(arr.dtype, np.floating) + ): + raise TypeError( + f"barriers must contain numeric cell values, " + f"got dtype {arr.dtype} from {barriers!r}" + ) + return arr.astype(np.float64) + + +def _validate_search_radius(search_radius): + """Raise if *search_radius* is not None or a non-negative integer.""" + if search_radius is None: + return + if not isinstance(search_radius, (int, np.integer)): + raise TypeError( + f"search_radius must be a non-negative integer or None, " + f"got {type(search_radius).__name__} {search_radius!r}" + ) + if search_radius < 0: + raise ValueError( + f"search_radius must be non-negative, got {search_radius}" + ) + + +def _validate_point(point, name, func_name): + """Raise ValueError if *point* is not a 2-element (y, x) pair.""" + try: + n = len(point) + except TypeError: + n = None + if n != 2: + raise ValueError( + f"{func_name}(): `{name}` must have exactly 2 elements (y, x), " + f"got {point!r}" + ) + + def _get_pixel_id(point, raster, xdim=None, ydim=None): # get location in `raster` pixel space for `point` in y-x coordinate space # point: (y, x) - coordinates of the point @@ -930,13 +995,15 @@ def a_star_search(surface: xr.DataArray, _validate_raster(friction, func_name='a_star_search', name='friction', ndim=2) - if surface.dims != (y, x): - raise ValueError("`surface.coords` should be named as coordinates:" - "({}, {})".format(y, x)) + _validate_surface_dims(surface, x, y, 'a_star_search') if connectivity != 4 and connectivity != 8: raise ValueError("Use either 4 or 8-connectivity.") + _validate_search_radius(search_radius) + _validate_point(start, 'start', 'a_star_search') + _validate_point(goal, 'goal', 'a_star_search') + # Detect backend surface_data = surface.data _is_dask = da is not None and isinstance(surface_data, da.Array) @@ -964,7 +1031,7 @@ def a_star_search(surface: xr.DataArray, if not _is_inside(goal_py, goal_px, h, w): raise ValueError("goal location outside the surface graph.") - barriers = np.asarray(barriers) + barriers = _validate_barriers(barriers) # --- Snap / crossability checks --- if _is_dask: @@ -1425,6 +1492,8 @@ def multi_stop_search(surface: xr.DataArray, _validate_raster(friction, func_name='multi_stop_search', name='friction', ndim=2) + _validate_surface_dims(surface, x, y, 'multi_stop_search') + if len(waypoints) < 2: raise ValueError("at least 2 waypoints are required") @@ -1436,9 +1505,7 @@ def multi_stop_search(surface: xr.DataArray, ) for idx, wp in enumerate(waypoints): - if len(wp) != 2: - raise ValueError( - f"waypoint {idx} must have exactly 2 elements (y, x)") + _validate_point(wp, f'waypoint {idx}', 'multi_stop_search') h, w = surface.shape for idx, wp in enumerate(waypoints): diff --git a/xrspatial/tests/test_pathfinding.py b/xrspatial/tests/test_pathfinding.py index 0db74e791..68a2a6b4f 100644 --- a/xrspatial/tests/test_pathfinding.py +++ b/xrspatial/tests/test_pathfinding.py @@ -1195,3 +1195,119 @@ def test_multi_stop_caps_waypoints(self): too_many = [(i % 10, (i * 7) % 10) for i in range(_MAX_WAYPOINTS + 1)] with pytest.raises(ValueError, match=f"at most {_MAX_WAYPOINTS}"): multi_stop_search(s, too_many) + + +class TestPathfindingErrorHandling: + """Error-handling sweep findings: barriers, search_radius, points, dims (#3649).""" + + @staticmethod + def _wall_surface(): + import xarray as xr + data = np.ones((5, 9)) + data[:, 4] = 0.0 # wall of zeros across the middle column + r = xr.DataArray(data, dims=('y', 'x'), attrs={'res': (1.0, 1.0)}) + r['y'] = np.linspace(4, 0, 5) + r['x'] = np.linspace(0, 8, 9) + return r + + # --- barriers --- + + def test_string_barriers_raise_instead_of_silently_ignored(self): + # barriers=['0'] used to route straight through a wall that + # barriers=[0] blocks, with no error or warning + r = self._wall_surface() + with pytest.raises(TypeError, match="barriers must contain numeric"): + a_star_search(r, (2.0, 0.0), (2.0, 8.0), barriers=['0']) + + def test_scalar_barriers_raise_with_hint(self): + # barriers=0 used to die with a numba TypingError deep in the kernel + r = self._wall_surface() + with pytest.raises(ValueError, match=r"1-D.*barriers=\[0\]"): + a_star_search(r, (2.0, 0.0), (2.0, 8.0), barriers=0) + + def test_numeric_barriers_still_block(self): + r = self._wall_surface() + path = a_star_search(r, (2.0, 0.0), (2.0, 8.0), + barriers=np.array([0])) + assert not np.isfinite(path.values).any() + + @pytest.mark.skipif(not has_dask_array(), reason="Requires dask.Array") + def test_string_barriers_raise_on_dask(self): + r = self._wall_surface() + r.data = da.from_array(r.data, chunks=(3, 3)) + with pytest.raises(TypeError, match="barriers must contain numeric"): + a_star_search(r, (2.0, 0.0), (2.0, 8.0), barriers=['0']) + + # --- search_radius --- + + def test_negative_search_radius_raises(self): + # used to silently return all-NaN ("no path") though a path exists + r = self._wall_surface() + with pytest.raises(ValueError, match="search_radius must be non-negative"): + a_star_search(r, (2.0, 0.0), (2.0, 8.0), search_radius=-1) + + def test_float_search_radius_raises(self): + # used to crash with "slice indices must be integers" for some + # start/goal geometries and work for others + r = self._wall_surface() + with pytest.raises(TypeError, match="search_radius must be a non-negative integer"): + a_star_search(r, (2.0, 0.0), (2.0, 8.0), search_radius=2.5) + + def test_zero_search_radius_accepted(self): + r = self._wall_surface() + path = a_star_search(r, (2.0, 0.0), (2.0, 2.0), search_radius=0) + assert np.isfinite(path.values[2, 2]) + + def test_negative_search_radius_raises_in_multi_stop(self): + # used to surface as a misleading "no path between waypoints 0 and 1" + r = self._wall_surface() + with pytest.raises(ValueError, match="search_radius must be non-negative"): + multi_stop_search(r, [(2.0, 0.0), (2.0, 2.0)], search_radius=-1) + + # --- start / goal --- + + def test_scalar_start_raises(self): + # used to raise "'float' object is not subscriptable" in _get_pixel_id + r = self._wall_surface() + with pytest.raises(ValueError, match="`start` must have exactly 2 elements"): + a_star_search(r, 3.0, (2.0, 2.0)) + + def test_three_element_goal_raises(self): + # used to silently drop the extra element + r = self._wall_surface() + with pytest.raises(ValueError, match="`goal` must have exactly 2 elements"): + a_star_search(r, (2.0, 0.0), (2.0, 2.0, 7.0)) + + def test_scalar_waypoint_raises(self): + # used to raise "object of type 'float' has no len()" + r = self._wall_surface() + with pytest.raises(ValueError, match="`waypoint 1` must have exactly 2 elements"): + multi_stop_search(r, [(2.0, 0.0), 3.0]) + + # --- dims consistency --- + + @staticmethod + def _latlon_surface(): + import xarray as xr + r = xr.DataArray(np.ones((5, 5)), dims=('lat', 'lon'), + attrs={'res': (1.0, 1.0)}) + r['lat'] = np.linspace(4, 0, 5) + r['lon'] = np.linspace(0, 4, 5) + return r + + def test_a_star_dims_mismatch_names_actual_dims(self): + r = self._latlon_surface() + with pytest.raises(ValueError, match=r"got \('lat', 'lon'\)"): + a_star_search(r, (2.0, 0.0), (2.0, 4.0)) + + def test_multi_stop_dims_mismatch_raises_value_error(self): + # used to raise a bare KeyError: 'y' from inside xarray + r = self._latlon_surface() + with pytest.raises(ValueError, match="expected `surface` to have dims"): + multi_stop_search(r, [(2.0, 0.0), (2.0, 4.0)]) + + def test_multi_stop_custom_dims_still_work(self): + r = self._latlon_surface() + result = multi_stop_search(r, [(2.0, 0.0), (2.0, 4.0)], + x='lon', y='lat') + assert np.isfinite(result.values).any() From 0c8c517901e859996d5d85714e12af5450dcc75c Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 8 Jul 2026 17:30:12 -0400 Subject: [PATCH 2/2] Address review: validate barriers at the multi_stop boundary, document constraints (#3649) Claude-Session: https://claude.ai/code/session_0155N4QGamQVxgpAAPbpQNq4 --- xrspatial/pathfinding.py | 13 ++++++++++++- xrspatial/tests/test_pathfinding.py | 7 +++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/xrspatial/pathfinding.py b/xrspatial/pathfinding.py index 4a6044c40..792752c77 100644 --- a/xrspatial/pathfinding.py +++ b/xrspatial/pathfinding.py @@ -56,6 +56,8 @@ def _validate_barriers(barriers): f"barriers must be a 1-D list or array of cell values, " f"got {arr.ndim}-D input.{hint}" ) + # bool arrays are rejected on purpose: barrier entries are cell + # values, and [True] is far more likely a mistake than "cells == 1" if arr.size > 0 and not ( np.issubdtype(arr.dtype, np.integer) or np.issubdtype(arr.dtype, np.floating) @@ -924,7 +926,8 @@ def a_star_search(surface: xr.DataArray, bounds raises a ``ValueError``. barriers : array like object, default=[] List of values inside the surface which are barriers - (cannot cross). + (cannot cross). Must be a 1-D sequence of numeric values; + anything else raises before the search runs. x : str, default='x' Name of the x coordinate in input surface raster. y: str, default='y' @@ -945,6 +948,7 @@ def a_star_search(surface: xr.DataArray, search_radius : int, optional Limit the A* search to a bounding box of ``±search_radius`` pixels around the start and goal. + Must be a non-negative integer (or ``None``). Dramatically reduces memory for large grids when start and goal are relatively close. If ``None`` (default) and the full grid would exceed 50 % of available RAM, an automatic @@ -1483,6 +1487,9 @@ def multi_stop_search(surface: xr.DataArray, If the surface is not 2-D, fewer than two waypoints are given, waypoints fall outside the surface bounds, or a segment is unreachable. + TypeError + If *barriers* contains non-numeric values or *search_radius* + is not an integer. """ # --- Input validation --- _validate_raster(surface, func_name='multi_stop_search', @@ -1494,6 +1501,10 @@ def multi_stop_search(surface: xr.DataArray, _validate_surface_dims(surface, x, y, 'multi_stop_search') + # fail at the API boundary rather than inside the first segment's + # a_star_search call (re-validation there is idempotent) + barriers = _validate_barriers(barriers) + if len(waypoints) < 2: raise ValueError("at least 2 waypoints are required") diff --git a/xrspatial/tests/test_pathfinding.py b/xrspatial/tests/test_pathfinding.py index 68a2a6b4f..050fa90c6 100644 --- a/xrspatial/tests/test_pathfinding.py +++ b/xrspatial/tests/test_pathfinding.py @@ -1231,6 +1231,13 @@ def test_numeric_barriers_still_block(self): barriers=np.array([0])) assert not np.isfinite(path.values).any() + def test_string_barriers_raise_in_multi_stop(self): + # validated at the multi_stop boundary, not inside the first + # a_star_search segment call + r = self._wall_surface() + with pytest.raises(TypeError, match="barriers must contain numeric"): + multi_stop_search(r, [(2.0, 0.0), (2.0, 2.0)], barriers=['0']) + @pytest.mark.skipif(not has_dask_array(), reason="Requires dask.Array") def test_string_barriers_raise_on_dask(self): r = self._wall_surface()