From 7d0fe310b5dbf7f73e96c67cebdb26653050607b Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Tue, 2 Jun 2026 14:51:31 -0700 Subject: [PATCH 1/2] Validate viewshed max_distance: reject negative and non-finite values (#2855) A negative, NaN, or infinite max_distance fell through the windowing math and raised confusing internal errors that never mentioned the argument. Validate at the public viewshed() entry point, before backend dispatch, so all four backends are covered, and raise a clear ValueError. Add tests for the rejected values and confirm finite values >= 0 still work. --- xrspatial/tests/test_viewshed.py | 25 +++++++++++++++++++++++++ xrspatial/viewshed.py | 20 ++++++++++++++++---- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/xrspatial/tests/test_viewshed.py b/xrspatial/tests/test_viewshed.py index b13978abe..098665854 100644 --- a/xrspatial/tests/test_viewshed.py +++ b/xrspatial/tests/test_viewshed.py @@ -580,6 +580,31 @@ def test_viewshed_custom_name(backend): assert result.name == "my_vs" +@pytest.mark.parametrize("bad", [-1.0, -0.5, float("nan"), + float("inf"), float("-inf")]) +def test_viewshed_invalid_max_distance_raises(bad): + """Negative or non-finite max_distance raises a clear ValueError (#2855). + + Validation lives at the public entry point, before backend dispatch, + so a single numpy raster covers every backend. Previously these + values fell through to confusing internal errors (e.g. "zero-size + array to reduction operation minimum" or "cannot convert float NaN + to integer"). + """ + raster = _make_raster("numpy") + with pytest.raises(ValueError, match="max_distance must be a finite"): + viewshed(raster, x=3, y=2, observer_elev=1, max_distance=bad) + + +@pytest.mark.parametrize("backend", ["numpy", "dask+numpy"]) +@pytest.mark.parametrize("good", [0.0, 3.0]) +def test_viewshed_valid_max_distance_still_works(backend, good): + """Finite max_distance >= 0 passes validation and returns a result.""" + raster = _make_raster(backend) + result = viewshed(raster, x=3, y=2, observer_elev=1, max_distance=good) + assert result.shape == raster.shape + + # ------------------------------------------------------------------- # dask+cupy backend tests # ------------------------------------------------------------------- diff --git a/xrspatial/viewshed.py b/xrspatial/viewshed.py index 248d42caf..2c6905568 100644 --- a/xrspatial/viewshed.py +++ b/xrspatial/viewshed.py @@ -1637,10 +1637,12 @@ def viewshed(raster: xarray.DataArray, when it is being analyzed for visibility. max_distance : float, optional Maximum analysis distance from the observer in surface units. - Cells beyond this distance are marked INVISIBLE without being - evaluated. When set and the raster is dask-backed, only the - chunks within the distance window are loaded — this is the most - efficient way to run viewshed on very large dask rasters. + Must be a finite number >= 0; a negative or non-finite value + raises ``ValueError``. Cells beyond this distance are marked + INVISIBLE without being evaluated. When set and the raster is + dask-backed, only the chunks within the distance window are + loaded — this is the most efficient way to run viewshed on very + large dask rasters. name : str, default='viewshed' Name of the output DataArray. Set on every backend so the result name does not depend on which backend ran. @@ -1716,6 +1718,16 @@ def viewshed(raster: xarray.DataArray, """ _validate_raster(raster, func_name='viewshed', name='raster') + if max_distance is not None: + try: + is_bad = not np.isfinite(max_distance) or max_distance < 0 + except (TypeError, ValueError): + is_bad = True + if is_bad: + raise ValueError( + "max_distance must be a finite number >= 0, " + f"got {max_distance!r}") + # --- max_distance: extract spatial window for any backend --- if max_distance is not None: return _viewshed_windowed(raster, x, y, observer_elev, target_elev, From 9104c382a9c095de1ea15f51fb91555a4208443d Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Tue, 2 Jun 2026 14:53:09 -0700 Subject: [PATCH 2/2] Fold max_distance validation into the windowing branch (#2855) Address review nit: merge the two consecutive 'if max_distance is not None' blocks so the check and the windowed dispatch share one branch. --- xrspatial/viewshed.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/xrspatial/viewshed.py b/xrspatial/viewshed.py index 2c6905568..c2b4ad064 100644 --- a/xrspatial/viewshed.py +++ b/xrspatial/viewshed.py @@ -1718,6 +1718,7 @@ def viewshed(raster: xarray.DataArray, """ _validate_raster(raster, func_name='viewshed', name='raster') + # --- max_distance: validate, then extract spatial window for any backend --- if max_distance is not None: try: is_bad = not np.isfinite(max_distance) or max_distance < 0 @@ -1727,9 +1728,6 @@ def viewshed(raster: xarray.DataArray, raise ValueError( "max_distance must be a finite number >= 0, " f"got {max_distance!r}") - - # --- max_distance: extract spatial window for any backend --- - if max_distance is not None: return _viewshed_windowed(raster, x, y, observer_elev, target_elev, max_distance, name)