From 8cba15e9bf3f916d535ad04f64fe30d36651779b Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Sun, 3 May 2026 16:05:29 -0700 Subject: [PATCH] Validate cellsize in hydro public APIs (#1429) twi_d8, flow_direction_d8/_dinf/_mfd, and flow_length_d8/_dinf/_mfd all call get_dataarray_resolution() and divide by the result without checking that the cellsize is finite and non-zero. A 1-D-coord DataArray (or one with NaN coords) silently produced inf or zero output. Add a finite-and-non-zero check at each of the seven call sites that raises ValueError with a message naming the cellsize values. 7 new tests in test_validate_cellsize.py covering each function. --- xrspatial/hydro/flow_direction_d8.py | 8 ++ xrspatial/hydro/flow_direction_dinf.py | 8 ++ xrspatial/hydro/flow_direction_mfd.py | 8 ++ xrspatial/hydro/flow_length_d8.py | 8 ++ xrspatial/hydro/flow_length_dinf.py | 8 ++ xrspatial/hydro/flow_length_mfd.py | 8 ++ .../hydro/tests/test_validate_cellsize.py | 113 ++++++++++++++++++ xrspatial/hydro/twi_d8.py | 8 ++ 8 files changed, 169 insertions(+) create mode 100644 xrspatial/hydro/tests/test_validate_cellsize.py diff --git a/xrspatial/hydro/flow_direction_d8.py b/xrspatial/hydro/flow_direction_d8.py index 5f57cb70..80bf2c01 100644 --- a/xrspatial/hydro/flow_direction_d8.py +++ b/xrspatial/hydro/flow_direction_d8.py @@ -314,6 +314,14 @@ def flow_direction_d8(agg: xr.DataArray, _validate_boundary(boundary) cellsize_x, cellsize_y = get_dataarray_resolution(agg) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"flow_direction(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure agg has at least 2 cells per spatial dimension " + f"with finite coords." + ) mapper = ArrayTypeFunctionMapping( numpy_func=_run_numpy, diff --git a/xrspatial/hydro/flow_direction_dinf.py b/xrspatial/hydro/flow_direction_dinf.py index a1e69757..712d32b1 100644 --- a/xrspatial/hydro/flow_direction_dinf.py +++ b/xrspatial/hydro/flow_direction_dinf.py @@ -416,6 +416,14 @@ def flow_direction_dinf(agg: xr.DataArray, _validate_boundary(boundary) cellsize_x, cellsize_y = get_dataarray_resolution(agg) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"flow_direction_dinf(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure agg has at least 2 cells per spatial dimension " + f"with finite coords." + ) mapper = ArrayTypeFunctionMapping( numpy_func=_run_numpy, diff --git a/xrspatial/hydro/flow_direction_mfd.py b/xrspatial/hydro/flow_direction_mfd.py index eb9c5f2d..7e0cfe61 100644 --- a/xrspatial/hydro/flow_direction_mfd.py +++ b/xrspatial/hydro/flow_direction_mfd.py @@ -392,6 +392,14 @@ def flow_direction_mfd(agg: xr.DataArray, p_fixed = -1.0 # sentinel for adaptive mode cellsize_x, cellsize_y = get_dataarray_resolution(agg) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"flow_direction_mfd(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure agg has at least 2 cells per spatial dimension " + f"with finite coords." + ) mapper = ArrayTypeFunctionMapping( numpy_func=_run_numpy, diff --git a/xrspatial/hydro/flow_length_d8.py b/xrspatial/hydro/flow_length_d8.py index 9a579dd7..8ef9e1d6 100644 --- a/xrspatial/hydro/flow_length_d8.py +++ b/xrspatial/hydro/flow_length_d8.py @@ -1136,6 +1136,14 @@ def flow_length_d8(flow_dir: xr.DataArray, f"direction must be 'downstream' or 'upstream', got {direction!r}") cellsize_x, cellsize_y = get_dataarray_resolution(flow_dir) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"flow_length(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure flow_dir has at least 2 cells per spatial dimension " + f"with finite coords." + ) cellsize_x = abs(cellsize_x) cellsize_y = abs(cellsize_y) diag = np.sqrt(cellsize_x ** 2 + cellsize_y ** 2) diff --git a/xrspatial/hydro/flow_length_dinf.py b/xrspatial/hydro/flow_length_dinf.py index 4eaf3acc..7821c3a7 100644 --- a/xrspatial/hydro/flow_length_dinf.py +++ b/xrspatial/hydro/flow_length_dinf.py @@ -990,6 +990,14 @@ def flow_length_dinf(flow_dir: xr.DataArray, f"direction must be 'downstream' or 'upstream', got {direction!r}") cellsize_x, cellsize_y = get_dataarray_resolution(flow_dir) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"flow_length_dinf(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure flow_dir has at least 2 cells per spatial dimension " + f"with finite coords." + ) cellsize_x = abs(cellsize_x) cellsize_y = abs(cellsize_y) diff --git a/xrspatial/hydro/flow_length_mfd.py b/xrspatial/hydro/flow_length_mfd.py index ba9a2265..d207b81e 100644 --- a/xrspatial/hydro/flow_length_mfd.py +++ b/xrspatial/hydro/flow_length_mfd.py @@ -1027,6 +1027,14 @@ def flow_length_mfd(flow_dir_mfd: xr.DataArray, ) cellsize_x, cellsize_y = get_dataarray_resolution(flow_dir_mfd) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"flow_length_mfd(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure flow_dir_mfd has at least 2 cells per spatial dimension " + f"with finite coords." + ) cellsize_x = abs(cellsize_x) cellsize_y = abs(cellsize_y) diff --git a/xrspatial/hydro/tests/test_validate_cellsize.py b/xrspatial/hydro/tests/test_validate_cellsize.py new file mode 100644 index 00000000..919909e5 --- /dev/null +++ b/xrspatial/hydro/tests/test_validate_cellsize.py @@ -0,0 +1,113 @@ +"""Tests for issue #1429: hydro cellsize validation. + +Functions that divide by `get_dataarray_resolution()` previously +silently produced inf/-inf/zero output for degenerate input coords +(1-D coordinates resolving to zero spacing, NaN coords, etc.). +""" + +import numpy as np +import pytest +import xarray as xr + +from xrspatial.hydro import ( + flow_direction_d8, + flow_direction_dinf, + flow_direction_mfd, + flow_length_d8, + flow_length_dinf, + flow_length_mfd, + twi_d8, +) + + +def _zero_cellsize_raster(shape=(5, 5)): + """Build a DataArray whose coords resolve to zero cellsize. + + A length-1 coordinate axis means there is no diff to compute. + """ + data = np.ones(shape, dtype=np.float64) + return xr.DataArray( + data, + dims=('y', 'x'), + coords={ + 'y': np.zeros(shape[0], dtype=np.float64), + 'x': np.zeros(shape[1], dtype=np.float64), + }, + attrs={'res': (0.0, 0.0)}, + ) + + +# --------------------------------------------------------------------------- +# flow_direction_* +# --------------------------------------------------------------------------- + +class TestFlowDirectionCellsize: + def test_d8_rejects_zero_cellsize(self): + with pytest.raises(ValueError, match="cellsize"): + flow_direction_d8(_zero_cellsize_raster()) + + def test_dinf_rejects_zero_cellsize(self): + with pytest.raises(ValueError, match="cellsize"): + flow_direction_dinf(_zero_cellsize_raster()) + + def test_mfd_rejects_zero_cellsize(self): + with pytest.raises(ValueError, match="cellsize"): + flow_direction_mfd(_zero_cellsize_raster()) + + +# --------------------------------------------------------------------------- +# flow_length_* +# --------------------------------------------------------------------------- + +class TestFlowLengthCellsize: + def test_d8_rejects_zero_cellsize(self): + fd = xr.DataArray( + np.full((5, 5), 1.0, dtype=np.float64), + dims=('y', 'x'), + coords={ + 'y': np.zeros(5, dtype=np.float64), + 'x': np.zeros(5, dtype=np.float64), + }, + attrs={'res': (0.0, 0.0)}, + ) + with pytest.raises(ValueError, match="cellsize"): + flow_length_d8(fd) + + def test_dinf_rejects_zero_cellsize(self): + fd = xr.DataArray( + np.full((5, 5), 0.0, dtype=np.float64), + dims=('y', 'x'), + coords={ + 'y': np.zeros(5, dtype=np.float64), + 'x': np.zeros(5, dtype=np.float64), + }, + attrs={'res': (0.0, 0.0)}, + ) + with pytest.raises(ValueError, match="cellsize"): + flow_length_dinf(fd) + + def test_mfd_rejects_zero_cellsize(self): + fractions = xr.DataArray( + np.full((8, 5, 5), 0.125, dtype=np.float64), + dims=('neighbor', 'y', 'x'), + coords={ + 'neighbor': ['E', 'SE', 'S', 'SW', 'W', 'NW', 'N', 'NE'], + 'y': np.zeros(5, dtype=np.float64), + 'x': np.zeros(5, dtype=np.float64), + }, + attrs={'res': (0.0, 0.0)}, + ) + with pytest.raises(ValueError, match="cellsize"): + flow_length_mfd(fractions) + + +# --------------------------------------------------------------------------- +# twi_d8 +# --------------------------------------------------------------------------- + +class TestTwiCellsize: + def test_rejects_zero_cellsize(self): + fa = _zero_cellsize_raster() + slope = _zero_cellsize_raster() + with pytest.raises(ValueError, match="cellsize"): + twi_d8(fa, slope) diff --git a/xrspatial/hydro/twi_d8.py b/xrspatial/hydro/twi_d8.py index 0af3cbe9..390ef8a0 100644 --- a/xrspatial/hydro/twi_d8.py +++ b/xrspatial/hydro/twi_d8.py @@ -56,6 +56,14 @@ def twi_d8(flow_accum: xr.DataArray, _validate_raster(slope_agg, func_name='twi', name='slope_agg') cellsize_x, cellsize_y = get_dataarray_resolution(flow_accum) + if not (np.isfinite(cellsize_x) and cellsize_x != 0 + and np.isfinite(cellsize_y) and cellsize_y != 0): + raise ValueError( + f"twi(): cellsize must be finite and non-zero " + f"(got cellsize_x={cellsize_x}, cellsize_y={cellsize_y}). " + f"Ensure flow_accum has at least 2 cells per spatial dimension " + f"with finite coords." + ) cellsize = np.sqrt(abs(cellsize_x * cellsize_y)) fa_data = flow_accum.data