Skip to content

Fix flake8 E201 in edge_detection kernel literals (#3675)#3679

Open
brendancol wants to merge 1 commit into
mainfrom
deep-sweep-style-edge_detection-2026-07-18
Open

Fix flake8 E201 in edge_detection kernel literals (#3675)#3679
brendancol wants to merge 1 commit into
mainfrom
deep-sweep-style-edge_detection-2026-07-18

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3675

Style cleanup from a /sweep-style pass over xrspatial/edge_detection.py. Categories addressed: Cat 1 (E201, E302) and Cat 4 (isort, test file only).

  • Drop the space after [ in the SOBEL_Y and PREWITT_Y kernel literals (the 4 E201 lines flake8 reports on this file). Matches the LAPLACIAN_KERNEL style already in the file; kernels are numerically identical.
  • test_edge_detection.py: add the missing second blank line before the first fixture (E302) and apply isort's import-block reflow for the configured line_length=100.

No behavior change. The module only wraps convolve_2d, so backend coverage is untouched (numpy / cupy / dask+numpy / dask+cupy all go through convolution).

Test plan:

  • flake8 xrspatial/edge_detection.py xrspatial/tests/test_edge_detection.py clean
  • isort --check-only clean on both files
  • 87 tests in test_edge_detection.py pass locally (CUDA host, GPU tests run)
  • Kernel arrays verified equal to their previous values

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: Fix flake8 E201 in edge_detection kernel literals (#3675)

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • xrspatial/edge_detection.py:12-14 and 21-23: dropping the leading space loses the first-column alignment that lined the 0/1 rows up under the -1 rows. A per-line # noqa: E201 would have kept it, but the new form matches LAPLACIAN_KERNEL a few lines down, so the file is at least internally consistent now. Fine as is.

What looks good

  • The kernel edits are whitespace-only. SOBEL_Y and PREWITT_Y still equal their previous values, and none of the five public functions changed.
  • The test-file import reflow is what the configured isort (line_length=100) produces, and the E302 fix takes the file to zero flake8 findings.
  • No runtime code paths are touched. All four backends route through convolve_2d, which this PR leaves alone.
  • State CSV row for edge_detection is one physical line and follows the existing schema.

Checklist

  • Algorithm matches reference (kernels numerically unchanged)
  • All implemented backends produce consistent results (no runtime change)
  • NaN handling is correct (untouched)
  • Edge cases are covered by tests (existing 87 tests pass)
  • Dask chunk boundaries handled correctly (untouched)
  • No premature materialization or unnecessary copies
  • Benchmark exists or is not needed (style-only change)
  • README feature matrix updated (not applicable, no new functions)
  • Docstrings present and accurate (unchanged)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Style: flake8 E201 in edge_detection kernel literals

1 participant