Skip to content

Style cleanup: flake8 + isort across xrspatial.geotiff subpackage#2285

Merged
brendancol merged 2 commits into
mainfrom
issue-2282
May 22, 2026
Merged

Style cleanup: flake8 + isort across xrspatial.geotiff subpackage#2285
brendancol merged 2 commits into
mainfrom
issue-2282

Conversation

@brendancol
Copy link
Copy Markdown
Contributor

Summary

Closes #2282.

Behaviour-neutral cleanup of every flake8 and isort report against xrspatial/geotiff/. Starting count: 309 flake8 violations, 22 isort-out-of-order files. Final: both tools clean.

Categories addressed

  • Cat 1 (E-codes): continuation indents (E127/E128), blank-line spacing (E302/E303/E305), nested-def blank line (E306), module-level-import (E402) with # noqa for pytest.importorskip and late-binding cases, long-line (E501) with # noqa for XML fixtures and codec lookup tables.
  • Cat 2 (W391): trimmed trailing blank lines at EOF in four files.
  • Cat 3 (F-codes): F401 unused imports deleted; F811 redefinitions cleaned; F841 unused locals removed; F821 cupy annotation fixed via TYPE_CHECKING import; F541 f-string without placeholders converted.
  • Cat 4 (isort): re-ordered imports across 60+ files using the project's own isort config (line_length=100).

Behaviour-changing fixes (Cat 3)

Each F841 was a leftover, not a real bug. All are deletions, no logic added:

  • _geotags.py geokeys = []: pre-refactor scratch list, the real one is key_entries.
  • _gpu_decode.py n_tiles = len(d_tile_bufs): re-bound from local later.
  • tests/test_features.py dtype_np, tiles_per_band: scratch locals.
  • tests/test_security.py header = parse_header(base): computed but never asserted.
  • tests/test_edge_cases.py with open(path, 'wb') as f: f was unused; rebound to bare with open(...).

The F821 cupy undefined-name in _writers/gpu.py:71 is a type annotation only (xr.DataArray | cupy.ndarray | np.ndarray). Fixed by importing cupy under TYPE_CHECKING so pyflakes resolves the name without forcing a runtime import.

Re-export strategy for __init__.py

xrspatial.geotiff/__init__.py re-exports a long list of internal helpers under leading-underscore aliases (_coords_from_pixel_geometry, _transform_tuple, etc) that are part of the documented internal API surface. The existing convention at lines 89 and 119 already uses # noqa: F401. This PR extends the same convention to every other re-export line so pyflakes stops flagging them.

Out of scope

  • No flake8 or isort config changes in setup.cfg.
  • No autoformatter (black, ruff format, autopep8) was run.
  • No functional refactors.

Test plan

  • flake8 xrspatial/geotiff/ returns clean.
  • isort --check-only xrspatial/geotiff/ returns clean.
  • import xrspatial.geotiff succeeds; __all__ unchanged.
  • pytest xrspatial/geotiff/tests/test_header.py xrspatial/geotiff/tests/test_geotags.py xrspatial/geotiff/tests/test_edge_cases.py passes (140 tests).
  • pytest xrspatial/geotiff/tests/test_features.py -m "not slow" passes (126 passed, 4 skipped).

)

Behaviour-neutral cleanup of every flake8 and isort report against
xrspatial/geotiff/:

- Cat 1 E-codes: continuation indents (E127/E128) in _writer.py,
  _gpu_decode.py, _write_layout.py, _crs.py, _sources.py, several test
  files; blank-line spacing (E302/E303/E305); blank-before-nested-def
  (E306) in test_features.py; module-level-import-not-at-top (E402)
  hand-marked with # noqa where the import sits behind a
  pytest.importorskip or is a late binding by design; long-line E501
  marked # noqa on lines that are XML fixtures or codec lookup tables.
- Cat 2 W391: trimmed trailing blank lines at EOF in four files.
- Cat 3 F-codes:
  - F401 unused imports deleted where safe; intentional re-exports in
    __init__.py marked # noqa: F401 to preserve the documented
    xrspatial.geotiff._xxx surface.
  - F811 redefinitions cleaned up.
  - F841 unused locals removed (each was a leftover, no behaviour
    change): _geotags.py geokeys list, _gpu_decode.py n_tiles,
    test_features.py dtype_np/tiles_per_band, test_security.py header,
    test_edge_cases.py unused 'f' rebound to bare 'with'.
  - F821 cupy in _writers/gpu.py:71 fixed by importing cupy under
    TYPE_CHECKING so the type annotation resolves without a runtime
    cupy import.
  - F541 f-string without placeholders in _validation.py:312 converted
    to a regular string.
- Cat 4 isort: re-ordered imports in 60+ files via the project's own
  isort config (line_length=100). Two files kept structured multi-line
  imports manually because isort would otherwise collapse the inline
  category comments.

setup.cfg flake8/isort config was not changed. No autoformatter (black,
ruff format, autopep8) was run.

Smoke tests: test_header.py, test_geotags.py, test_edge_cases.py,
test_features.py pass.

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

Self-review notes for the style cleanup.

Scope check:

  • flake8 xrspatial/geotiff/ now exits 0 (was 309 violations).
  • isort --check-only xrspatial/geotiff/ now exits 0 (was 22 files out of order).
  • setup.cfg flake8/isort config unchanged.

Behaviour-touching changes are limited to:

  • F401 unused imports removed (verified each name is not referenced elsewhere in the file).
  • F841 unused locals removed (all leftover scratch variables, no assertions or side effects lost).
  • F821 cupy annotation fixed by adding cupy under TYPE_CHECKING in _writers/gpu.py. No runtime import added.
  • F541 f-string -> plain string in _validation.py:312.
  • F811 redefinitions cleaned (one in init.py was a nested import os inside a function; the module already imports os at the top).

Re-export surface preserved: every line in xrspatial.geotiff/init.py that re-exports a _xxx-prefixed internal helper now carries # noqa: F401 so pyflakes does not flag the alias. The existing convention already used this at lines 89 and 119.

E501 noqa was applied to: XML fixture lines in test_features.py and bench_vs_rioxarray.py, GeoTIFF metadata field descriptions in _write_layout.py, and the f'{"f" if ...}' codec dtype string in _vrt.py:1572 that resists clean line breaks.

E402 noqa applied to: imports that sit after a pytest.importorskip (test files) and the late from numba import int32 as numba_int32 set in _gpu_decode.py:317 where the import is intentionally placed below the device kernels that consume it.

Smoke tests:

  • test_header.py, test_geotags.py, test_edge_cases.py: 140 passed.
  • test_features.py (-m "not slow"): 126 passed, 4 skipped.

The merge with main is currently flagged as conflicting; resolving in Step 11.

# Conflicts:
#	xrspatial/geotiff/__init__.py
#	xrspatial/geotiff/_geotags.py
#	xrspatial/geotiff/_writer.py
@brendancol brendancol merged commit 695dc30 into main May 22, 2026
5 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: flake8 + isort across xrspatial.geotiff subpackage

1 participant