Skip to content

zonal: fix flake8 F401, E501 + isort drift#2524

Merged
brendancol merged 1 commit into
mainfrom
deep-sweep-style-zonal-2026-05-27
May 27, 2026
Merged

zonal: fix flake8 F401, E501 + isort drift#2524
brendancol merged 1 commit into
mainfrom
deep-sweep-style-zonal-2026-05-27

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Closes #2522.

Summary

  • Drop unused not_implemented_func import (F401, Cat 3).
  • Wrap the 117-char dd.concat one-liner at line 455 (E501, Cat 1).
  • Consolidate the xrspatial.utils import block — merge the trailing
    has_dask_array, alphabetise, trim the extra blank line above
    # 3rd-party (Cat 4, isort).

No behavioural change. not_implemented_func was only on the import
line; there are no call sites.

Test plan

  • flake8 xrspatial/zonal.py clean
  • isort --check-only xrspatial/zonal.py clean
  • pytest xrspatial/tests/test_zonal.py -x — 125 passed

- Cat 3 (F401): drop unused not_implemented_func import.
- Cat 1 (E501): wrap dd.concat one-liner at line 455.
- Cat 4 (isort): consolidate xrspatial.utils import block.

No behavioural change. flake8 and isort clean. All 125 zonal tests
pass locally.

Closes #2522
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label May 27, 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.

PR Review: zonal: fix flake8 F401, E501 + isort drift

Blockers

None.

Suggestions

None.

Nits

None.

What looks good

  • Diff matches the three findings in issue #2522: one F401, one E501, one isort consolidation. No drive-by edits.
  • not_implemented_func is unused. Grep across the file finds zero call sites, and grep across xrspatial/ finds no re-export consumer (__init__.py and accessor.py import specific functions from xrspatial.zonal, none of them not_implemented_func).
  • The dd.concat wrap preserves call semantics: same positional list, same axis=1, ignore_unknown_divisions=True kwargs.
  • The retained import block keeps all 9 names used elsewhere in the module (43 references across the file).
  • flake8 xrspatial/zonal.py and isort --check-only xrspatial/zonal.py both pass on the PR head.
  • 125 zonal tests pass locally on the PR head.

Checklist

  • Algorithm matches reference/paper (N/A, no algorithm change)
  • All implemented backends produce consistent results (N/A, no dispatch change)
  • NaN handling is correct (N/A)
  • Edge cases are covered by tests (N/A, existing tests cover behaviour)
  • Dask chunk boundaries handled correctly (N/A)
  • No premature materialization or unnecessary copies (N/A)
  • Benchmark exists or is not needed (N/A, pure style)
  • README feature matrix updated (N/A, no API change)
  • Docstrings present and accurate (N/A)

@brendancol brendancol merged commit a6fb701 into main May 27, 2026
9 checks passed
@brendancol
Copy link
Copy Markdown
Contributor Author

PR Review

Blockers

None.

Suggestions

None.

Nits

None.

What looks good

  • Scope matches the description: one F401, one E501 wrap, one isort consolidation.
  • not_implemented_func had no call sites in xrspatial/zonal.py; removal is safe and the symbol remains exported from xrspatial.utils for other modules.
  • The wrapped dd.concat at lines 451-454 keeps argument order and trailing-comma style consistent with the rest of the file.
  • flake8 xrspatial/zonal.py clean, isort --check-only xrspatial/zonal.py clean, all 125 tests in test_zonal.py pass on the branch.

Checklist

  • Algorithm matches reference (no algorithm change)
  • All implemented backends produce consistent results (no dispatch change)
  • NaN handling unchanged
  • Edge cases covered by existing tests
  • Dask chunk boundaries unchanged
  • No premature materialization introduced
  • Benchmark not needed (style-only)
  • README feature matrix not affected
  • Docstrings unchanged

Ready to merge.

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.

zonal: flake8 F401, E501 + isort drift

1 participant