From 0c4ed4e2ba74e0628d3fc3e9a06d041d31c5966d Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 3 Jun 2026 06:20:29 -0700 Subject: [PATCH 1/2] Validate planar cell size in slope() (#2897) --- xrspatial/slope.py | 13 +++++- xrspatial/tests/test_slope.py | 85 +++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/xrspatial/slope.py b/xrspatial/slope.py index 74bc06b8c..3ffdaf3e0 100644 --- a/xrspatial/slope.py +++ b/xrspatial/slope.py @@ -360,7 +360,9 @@ def slope(agg: xr.DataArray, size. If the coordinates are in degrees (lat/lon) but the elevation values are in meters, the result is wrong by orders of magnitude. When this mismatch is detected, a ``UserWarning`` is emitted suggesting you reproject - to a projected CRS or use ``method='geodesic'``. + to a projected CRS or use ``method='geodesic'``. The ``'planar'`` method also + raises a ``ValueError`` if the cell size on either axis is zero, negative, or + non-finite, since those values cannot produce a valid slope. References ---------- @@ -401,6 +403,15 @@ def slope(agg: xr.DataArray, if method == 'planar': warn_if_unit_mismatch(agg) cellsize_x, cellsize_y = get_dataarray_resolution(agg) + if (not np.isfinite(cellsize_x) or not np.isfinite(cellsize_y) + or cellsize_x <= 0 or cellsize_y <= 0): + raise ValueError( + "slope(method='planar') requires a positive, finite cell size " + f"on both axes; got cellsize_x={cellsize_x!r}, " + f"cellsize_y={cellsize_y!r}. " + "Set agg.attrs['res'] to a (x, y) tuple of positive floats, " + "or attach numeric x/y coordinates to the DataArray." + ) mapper = ArrayTypeFunctionMapping( numpy_func=_run_numpy, cupy_func=_run_cupy, diff --git a/xrspatial/tests/test_slope.py b/xrspatial/tests/test_slope.py index 57a5ef262..adb675c36 100644 --- a/xrspatial/tests/test_slope.py +++ b/xrspatial/tests/test_slope.py @@ -463,3 +463,88 @@ def test_center_nan_propagates_dask_cupy(): dask_cupy_agg = create_test_raster(data, backend='dask+cupy', attrs={'res': (1, 1)}, chunks=(3, 3)) assert_numpy_equals_dask_cupy(numpy_agg, dask_cupy_agg, slope, nan_edges=False) + + +# --------------------------------------------------------------------------- +# Planar cell-size validation (issue #2897) +# --------------------------------------------------------------------------- + +INVALID_RES = [ + (0, 0), # zero on both axes + (0, 1), # zero on x + (1, 0), # zero on y + (-1, 1), # negative on x + (1, -1), # negative on y + (np.inf, 1), # inf on x + (1, np.inf), # inf on y + (np.nan, 1), # nan on x + (1, np.nan), # nan on y +] + + +@pytest.mark.parametrize("res", INVALID_RES) +def test_planar_invalid_cellsize_numpy(res): + """slope(method='planar') must reject zero, negative, or non-finite cell + sizes with a clear ValueError instead of a raw ZeroDivisionError, silent + NaN interior, or plausible-but-wrong values. + """ + data = np.zeros((5, 5), dtype=np.float32) + agg = create_test_raster(data, backend='numpy', attrs={'res': res}) + with pytest.raises(ValueError, match="positive, finite cell size"): + slope(agg) + + +@dask_array_available +@pytest.mark.parametrize("res", INVALID_RES) +def test_planar_invalid_cellsize_dask_numpy(res): + data = np.zeros((5, 5), dtype=np.float32) + agg = create_test_raster(data, backend='dask+numpy', + attrs={'res': res}, chunks=(3, 3)) + with pytest.raises(ValueError, match="positive, finite cell size"): + slope(agg) + + +@cuda_and_cupy_available +@pytest.mark.parametrize("res", INVALID_RES) +def test_planar_invalid_cellsize_cupy(res): + data = np.zeros((5, 5), dtype=np.float32) + agg = create_test_raster(data, backend='cupy', attrs={'res': res}) + with pytest.raises(ValueError, match="positive, finite cell size"): + slope(agg) + + +@dask_array_available +@cuda_and_cupy_available +@pytest.mark.parametrize("res", INVALID_RES) +def test_planar_invalid_cellsize_dask_cupy(res): + data = np.zeros((5, 5), dtype=np.float32) + agg = create_test_raster(data, backend='dask+cupy', + attrs={'res': res}, chunks=(3, 3)) + with pytest.raises(ValueError, match="positive, finite cell size"): + slope(agg) + + +def test_planar_valid_cellsize_still_works(): + """A normal positive cell size must not trip the new guard.""" + data = np.zeros((5, 5), dtype=np.float32) + agg = create_test_raster(data, backend='numpy', attrs={'res': (10, 10)}) + result = slope(agg) + general_output_checks(agg, result) + + +def test_geodesic_unaffected_by_invalid_res_attr(): + """The geodesic path takes its scale from lat/lon coords, so a bogus 'res' + attribute must not trigger the planar cell-size guard. + """ + lat = np.linspace(40.0, 40.4, 5) + lon = np.linspace(-105.0, -104.6, 5) + data = np.zeros((5, 5), dtype=np.float64) + agg = xr.DataArray( + data, + coords={'y': lat, 'x': lon}, + dims=['y', 'x'], + attrs={'res': (0, 0)}, + ) + # Must not raise the planar guard; geodesic uses lat/lon coords. + result = slope(agg, method='geodesic') + general_output_checks(agg, result) From f1a5eab78cb97c49f3ca5ecfa3d47b677efe74f8 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Wed, 3 Jun 2026 06:22:22 -0700 Subject: [PATCH 2/2] Address review nit: document negative cell-size rejection in slope() (#2897) --- xrspatial/slope.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xrspatial/slope.py b/xrspatial/slope.py index 3ffdaf3e0..e8e42322b 100644 --- a/xrspatial/slope.py +++ b/xrspatial/slope.py @@ -403,6 +403,8 @@ def slope(agg: xr.DataArray, if method == 'planar': warn_if_unit_mismatch(agg) cellsize_x, cellsize_y = get_dataarray_resolution(agg) + # Reject negatives too (curvature() only checks == 0): a negative cell + # size would silently flip the slope sign rather than error out. if (not np.isfinite(cellsize_x) or not np.isfinite(cellsize_y) or cellsize_x <= 0 or cellsize_y <= 0): raise ValueError(