A documentation sweep of xrspatial/edge_detection.py turned up three docstring problems in the five public functions (sobel_x, sobel_y, prewitt_x, prewitt_y, laplacian). Doc-only fixes; no code behavior should change.
First, none of the five docstrings has an Examples section. There isn't a single >>> block in the module. Parameters and Returns are present and accurate.
Second, the gradient docstrings say e.g. "Detects vertical edges by convolving with the Sobel-X kernel" and show the kernel, but the implementation goes through convolve_2d, which applies kernels by cross-correlation (same as scipy.ndimage.correlate; the convolution_2d docstring says so explicitly). The Sobel and Prewitt kernels are antisymmetric, so convolution and correlation differ by sign. Someone who hand-computes the documented convolution gets the negated result:
import numpy as np, xarray as xr
from xrspatial import sobel_x
from scipy import ndimage
data = np.arange(30, dtype=np.float64).reshape(5, 6) % 6 # increases left->right
K = np.array([[-1, 0, 1], [-2, 0, 2], [-1, 0, 1]], float)
sobel_x(xr.DataArray(data, dims=['y', 'x'])).data[2, 2] # 8.0
ndimage.correlate(data, K)[2, 2] # 8.0
ndimage.convolve(data, K)[2, 2] # -8.0
The code matches the usual Sobel/Prewitt convention (positive where values increase toward +x / +y), so the wording is what needs fixing: say the kernel is applied by cross-correlation and state the sign convention. This affects the four gradient operators. laplacian has a symmetric kernel, so both operations agree there.
Third, a NaN anywhere in the input propagates to its whole 3x3 neighborhood in the output, independent of the boundary mode. TestNanHandling in test_edge_detection.py pins this, but the docstrings only describe the boundary edge fill and never mention it.
Two smaller things, worth folding into the same pass or skipping: the module has no module-level docstring, and integer input comes back float32 (convolve_2d promotes). No docstring makes a dtype claim, so nothing is wrong today.
The rest of the audit came back clean: docstring parameters match the signatures, all five functions are listed in docs/source/reference/focal.rst, and the "Supports NumPy, CuPy, Dask+NumPy, and Dask+CuPy" claim checks out on all four backends (run on a CUDA host).
A documentation sweep of
xrspatial/edge_detection.pyturned up three docstring problems in the five public functions (sobel_x,sobel_y,prewitt_x,prewitt_y,laplacian). Doc-only fixes; no code behavior should change.First, none of the five docstrings has an Examples section. There isn't a single
>>>block in the module. Parameters and Returns are present and accurate.Second, the gradient docstrings say e.g. "Detects vertical edges by convolving with the Sobel-X kernel" and show the kernel, but the implementation goes through
convolve_2d, which applies kernels by cross-correlation (same asscipy.ndimage.correlate; theconvolution_2ddocstring says so explicitly). The Sobel and Prewitt kernels are antisymmetric, so convolution and correlation differ by sign. Someone who hand-computes the documented convolution gets the negated result:The code matches the usual Sobel/Prewitt convention (positive where values increase toward +x / +y), so the wording is what needs fixing: say the kernel is applied by cross-correlation and state the sign convention. This affects the four gradient operators.
laplacianhas a symmetric kernel, so both operations agree there.Third, a NaN anywhere in the input propagates to its whole 3x3 neighborhood in the output, independent of the
boundarymode.TestNanHandlingin test_edge_detection.py pins this, but the docstrings only describe theboundaryedge fill and never mention it.Two smaller things, worth folding into the same pass or skipping: the module has no module-level docstring, and integer input comes back float32 (
convolve_2dpromotes). No docstring makes a dtype claim, so nothing is wrong today.The rest of the audit came back clean: docstring parameters match the signatures, all five functions are listed in
docs/source/reference/focal.rst, and the "Supports NumPy, CuPy, Dask+NumPy, and Dask+CuPy" claim checks out on all four backends (run on a CUDA host).