Skip to content

Validate input rasters in edge_detection public API (#1271)#1273

Open
brendancol wants to merge 2 commits intomainfrom
issue-1271
Open

Validate input rasters in edge_detection public API (#1271)#1273
brendancol wants to merge 2 commits intomainfrom
issue-1271

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

  • Add _validate_raster(agg) at the top of sobel_x, sobel_y, laplacian, prewitt_x, and prewitt_y. The sibling modules (aspect, slope, curvature, bilateral) already do this; edge_detection was the outlier.
  • Add tests for np.ndarray, 1-D DataArray, and 3-D DataArray inputs across all five functions. Non-DataArray now raises TypeError, wrong-ndim raises ValueError.
  • Record the inspection in .claude/sweep-security-state.json (severity MEDIUM, Cat 6).

Cat 6 finding from the latest security sweep. Numerical correctness was not affected because convolve_2d._promote_float already casts integer dtypes to float32. This is purely an error-message UX fix.

Closes #1271.

Test plan

  • pytest xrspatial/tests/test_edge_detection.py -x (87 passed)
  • New TestInputValidation class covers all five public functions across three failure modes

Add _validate_raster(agg) at the top of sobel_x, sobel_y, laplacian,
prewitt_x, and prewitt_y so non-DataArray inputs raise TypeError and
wrong-ndim DataArrays raise ValueError instead of failing later inside
numba or cupy with a confusing message.

Numerical correctness is unaffected (convolve_2d already casts integer
dtypes to float32). This is purely an error-message UX improvement.

Add tests covering numpy.ndarray, 1-D DataArray, and 3-D DataArray
inputs for all five functions.

Record the inspection in .claude/sweep-security-state.json with
severity_max=MEDIUM and categories_found=[6].
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Apr 25, 2026
@brendancol
Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: brendancol <433221+brendancol@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 26, 2026

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflict in .claude/sweep-security-state.json (commit 5b9f021). The conflict was between the edge_detection entry added in this PR and the diffusion entry added in origin/main — both are now included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

edge_detection public API skips _validate_raster

2 participants