Style cleanup in focal.py: unused import, isort, mutable default (#2731)#2737
Style cleanup in focal.py: unused import, isort, mutable default (#2731)#2737brendancol wants to merge 2 commits into
Conversation
…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.
brendancol
left a comment
brendancol
left a comment
There was a problem hiding this comment.
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 ofmean(), ahead of both the 3D_apply_per_bandbranch and the 2D loop, so both paths get the intended default. Behaviour is unchanged from the oldexcludes=[np.nan]default. - isort output wraps the
xrspatial.convolutionandxrspatial.utilsfrom-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_funcremoval is safe: no__all__, and nothing imports it fromxrspatial.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.
1 participant
Closes #2731
Style-only cleanup of
xrspatial/focal.py, flagged by the project's ownsetup.cfg(flake8, isort) and a mutable-default grep. From the deep-sweep style pass on the focal module.not_implemented_funcimport. No__all__, nothing imports it fromxrspatial.focal, so it wasn't a re-export.line_length=100. Mechanical, no functional change.mean(..., excludes=[np.nan], ...)becomesexcludes=None, resolved to[np.nan]in the body. The list was never mutated, so behaviour is identical. Addedtest_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.
meandispatches throughArrayTypeFunctionMapping; no path-specific logic touched.Test plan:
flake8 xrspatial/focal.pycleanisort --check-only xrspatial/focal.pyclean