Skip to content

Style cleanup in focal.py: unused import, isort, mutable default (#2731)#2737

Merged
brendancol merged 4 commits into
mainfrom
deep-sweep-style-focal-2026-05-29
Jun 1, 2026
Merged

Style cleanup in focal.py: unused import, isort, mutable default (#2731)#2737
brendancol merged 4 commits into
mainfrom
deep-sweep-style-focal-2026-05-29

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2731

Style-only cleanup of xrspatial/focal.py, flagged by the project's own setup.cfg (flake8, isort) and a mutable-default grep. From the deep-sweep style pass on the focal module.

  • Cat 3 (F401): removed the unused not_implemented_func import. No __all__, nothing imports it from xrspatial.focal, so it wasn't a re-export.
  • Cat 4 (isort): reordered and rewrapped the top-of-file import block to line_length=100. Mechanical, no functional change.
  • Cat 5 (mutable default): mean(..., excludes=[np.nan], ...) becomes excludes=None, resolved to [np.nan] in the body. The list was never mutated, so behaviour is identical. Added test_mean_default_excludes_does_not_leak.

No behavioural change is intended for the Cat 3/4 fixes. The Cat 5 fix is behaviour-preserving. No config widening, no autoformatter.

Backends: static source-level changes, uniform across numpy / cupy / dask+numpy / dask+cupy. mean dispatches through ArrayTypeFunctionMapping; no path-specific logic touched.

Test plan:

  • flake8 xrspatial/focal.py clean
  • isort --check-only xrspatial/focal.py clean
  • 115 focal tests pass (incl. GPU/dask mean tests, CUDA available)
  • new regression test passes

…efault (#2731)

- Cat 3 (F401): drop unused not_implemented_func import from xrspatial.utils
- Cat 4 (isort): reorder/rewrap the top-of-file import block to the project's
  line_length=100 config; no functional change
- Cat 5: replace mutable default excludes=[np.nan] in mean() with a None
  sentinel resolved to [np.nan] in the body; behaviour preserved (the list
  was never mutated), add test_mean_default_excludes_does_not_leak

No behavioural change for Cat 3/4. Cat 5 fix is behaviour-preserving.
No config widening, no autoformatter.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 30, 2026
Copy link
Copy Markdown
Contributor Author

@brendancol brendancol left a comment

Choose a reason for hiding this comment

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

Domain-aware review (deep-sweep style worker)

Scope check: the diff is style-only and matches the three findings in #2731.

Correctness:

  • The excludes is None -> [np.nan] resolution sits at the top of mean(), ahead of both the 3D _apply_per_band branch and the 2D loop, so both paths get the intended default. Behaviour is unchanged from the old excludes=[np.nan] default.
  • isort output wraps the xrspatial.convolution and xrspatial.utils from-imports onto continuation lines at the 100-char limit; flake8 is clean on the result (no E128/E501), so the wrapping is consistent with the project config.
  • not_implemented_func removal is safe: no __all__, and nothing imports it from xrspatial.focal.

Backends: changes are static source edits and apply uniformly across numpy / cupy / dask+numpy / dask+cupy. mean dispatches through ArrayTypeFunctionMapping; no per-backend path was touched. The full focal suite (115 tests, including the GPU and dask mean tests) passes with CUDA available.

Nit (non-blocking): test_mean_default_excludes_does_not_leak runs on data_random, which has no NaNs, so it proves None == [np.nan] equivalence and no cross-call leak but does not itself exercise the NaN-exclusion path. That path is already covered by the existing test_mean_* transfer-function tests, so I left it as is.

No blockers, no required changes.

@brendancol brendancol merged commit 266842b into main Jun 1, 2026
6 checks passed
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.

Style cleanup in focal.py: unused import, isort drift, mutable default arg

1 participant