diff --git a/.claude/sweep-error-handling-state.csv b/.claude/sweep-error-handling-state.csv new file mode 100644 index 000000000..c86df4360 --- /dev/null +++ b/.claude/sweep-error-handling-state.csv @@ -0,0 +1,2 @@ +module,last_inspected,issue,severity_max,categories_found,notes +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." diff --git a/xrspatial/convolution.py b/xrspatial/convolution.py index b1b08bfb7..f185d56b1 100644 --- a/xrspatial/convolution.py +++ b/xrspatial/convolution.py @@ -344,6 +344,34 @@ def custom_kernel(kernel): return kernel +def _validate_kernel(kernel, func_name='convolve_2d'): + """Validate a convolution kernel: a 2D array with odd side lengths. + + Duck-typed on ``ndim``/``shape`` so numpy and cupy kernels both pass. + Rejects up front so a malformed kernel raises a clear ValueError + instead of an inscrutable numba ``TypingError`` (or silent off-center + output for even side lengths). Mirrors ``custom_kernel``'s odd-shape + contract. + """ + if not hasattr(kernel, 'ndim') or not hasattr(kernel, 'shape'): + raise ValueError( + f"{func_name}(): `kernel` must be a 2D array with odd side " + f"lengths, got {type(kernel).__module__}." + f"{type(kernel).__qualname__}" + ) + if kernel.ndim != 2: + raise ValueError( + f"{func_name}(): `kernel` must be a 2D array, got {kernel.ndim}D " + f"with shape {tuple(kernel.shape)}" + ) + rows, cols = kernel.shape + if rows % 2 == 0 or cols % 2 == 0: + raise ValueError( + f"{func_name}(): `kernel` must have odd side lengths so it has a " + f"well-defined center, got shape {(rows, cols)}" + ) + + @jit(nopython=True, nogil=True) def _convolve_2d_numpy(data, kernel): # apply kernel to data image. @@ -483,6 +511,7 @@ def convolve_2d(data, kernel, boundary='nan'): agg = xr.DataArray(data) _validate_raster(agg, func_name='convolve_2d', ndim=2) _validate_boundary(boundary) + _validate_kernel(kernel, func_name='convolve_2d') mapper = ArrayTypeFunctionMapping( numpy_func=_convolve_2d_numpy_boundary, cupy_func=_convolve_2d_cupy, @@ -615,6 +644,7 @@ def convolution_2d(agg, kernel, name='convolution_2d', boundary='nan'): """ # wrapper of convolve_2d + _validate_raster(agg, func_name='convolution_2d', ndim=2) out = convolve_2d(agg.data, kernel, boundary) return xr.DataArray(out, name=name, diff --git a/xrspatial/tests/test_convolution.py b/xrspatial/tests/test_convolution.py index dff4af4a7..1ce72bd03 100644 --- a/xrspatial/tests/test_convolution.py +++ b/xrspatial/tests/test_convolution.py @@ -2,7 +2,9 @@ import pytest import xarray as xr -from xrspatial.convolution import circle_kernel, convolve_2d, custom_kernel +from xrspatial.convolution import ( + circle_kernel, convolution_2d, convolve_2d, custom_kernel, +) KERNEL = circle_kernel(1, 1, 1) @@ -48,3 +50,48 @@ def test_convolve_2d_accepts_float64(): # Centre cell is finite; edges are NaN by default boundary mode. assert np.isfinite(out[2, 2]) assert np.isnan(out[0, 0]) + + +DATA = np.arange(25, dtype=np.float64).reshape(5, 5) + + +@pytest.mark.parametrize("bad_kernel", [ + None, + np.ones(3, dtype=np.float64), # 1D + np.ones((3, 3, 3), dtype=np.float64), # 3D + [[0, 1, 0], [1, 1, 1], [0, 1, 0]], # python list, not an array +]) +def test_convolve_2d_rejects_bad_kernel(bad_kernel): + # Bad kernels used to crash deep in numba with a cryptic TypingError. + # convolve_2d must reject them up front with a clear message that names + # `kernel`. + with pytest.raises(ValueError, match="kernel"): + convolve_2d(DATA, bad_kernel) + + +@pytest.mark.parametrize("even_kernel", [ + np.ones((2, 2), dtype=np.float64), + np.ones((4, 4), dtype=np.float64), + np.ones((2, 3), dtype=np.float64), +]) +def test_convolve_2d_rejects_even_kernel(even_kernel): + # An even side length has no well-defined center; convolve_2d used to + # silently produce an off-center result. custom_kernel already rejects + # even kernels, so convolve_2d must too. + with pytest.raises(ValueError, match="odd"): + convolve_2d(DATA, even_kernel) + + +def test_convolution_2d_rejects_non_dataarray(): + # Passing a plain numpy array used to fail with an inscrutable + # "'memoryview' object has no attribute 'astype'"; validate up front. + with pytest.raises(TypeError, match="DataArray"): + convolution_2d(DATA, KERNEL) + + +def test_convolution_2d_accepts_dataarray(): + # Positive path unchanged. + agg = xr.DataArray(DATA, dims=['y', 'x']) + out = convolution_2d(agg, KERNEL) + assert isinstance(out, xr.DataArray) + assert out.shape == agg.shape