Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .claude/sweep-error-handling-state.csv
Original file line number Diff line number Diff line change
@@ -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."
30 changes: 30 additions & 0 deletions xrspatial/convolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
49 changes: 48 additions & 1 deletion xrspatial/tests/test_convolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Loading