From 743c74086597ff6c291594847f2b0163ea7bee27 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 15 Jun 2026 06:47:16 -0700 Subject: [PATCH] Style: fix flake8/isort findings in cost_distance.py (#3339) Categories addressed: - Cat 3 (F401): remove unused `from functools import partial` (refactor leftover; not re-exported) - Cat 5 (mutable default): public `cost_distance(target_values=[])` -> `None` sentinel normalized to `[]` in the body, mirroring proximity.py (#2725). Reassigned via np.asarray before any mutation, so behaviour is preserved. Adds regression tests covering default reuse and None. - Cat 1 (E302): two blank lines before `@ngjit _heap_push` - Cat 1 (E127): fix continuation-line over-indent in _cost_distance_dask_cupy and _cost_distance_dask_iterative signatures - Cat 4 (isort): dataset_support before utils; utils from-import reflowed to 100 cols No behavioural change for the Cat 1/3/4 fixes. The Cat 5 change preserves behaviour. flake8 and isort --check-only are clean on the module; 57 cost_distance tests pass. --- .claude/sweep-style-state.csv | 1 + xrspatial/cost_distance.py | 25 ++++++++------- xrspatial/tests/test_cost_distance.py | 44 +++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/.claude/sweep-style-state.csv b/.claude/sweep-style-state.csv index 1ed258dcb..34c9990d6 100644 --- a/.claude/sweep-style-state.csv +++ b/.claude/sweep-style-state.csv @@ -1,6 +1,7 @@ module,last_inspected,issue,severity_max,categories_found,notes aspect,2026-05-29,2683,MEDIUM,1,E402+E305 line 38: from xrspatial.geodesic import block sat below _geodesic_cuda_dims; moved up with top-of-file imports. E501 lines 219/263: wrapped two _run_gpu_geodesic_aspect kernel-launch calls (101/109 chars). Cat 4 isort reviewed but NOT applied: slope.py/curvature.py use one-import-per-line for xrspatial.utils so raw isort would make aspect inconsistent. Cat 2/3/5 grep clean. PR #2740. 82 aspect+geodesic tests pass. contour,2026-05-29,2698,HIGH,3,"F821 line 557: contours() return annotation ""gpd.GeoDataFrame"" referenced gpd not bound at module scope (only imported inside _to_geopandas). Fixed via TYPE_CHECKING-guarded import geopandas as gpd, matching polygonize.py. No runtime change; geopandas stays optional. isort clean. Cat 1/2/4/5 clean. 24 contour tests pass. PR open." +cost_distance,2026-06-15,3339,HIGH,1;3;4;5,"Cat 3 F401: removed unused 'from functools import partial' (L32, refactor leftover, not re-exported). Cat 5: mutable default target_values: list = [] in public cost_distance() -> None sentinel normalized to [] in body (mirrors proximity.py #2725); reassigned via np.asarray before any mutation so behaviour preserved; added test_default_target_values_does_not_leak + test_target_values_none_matches_empty_default. Cat 1: E302 L63 (1->2 blank lines before @ngjit _heap_push; placed blanks between comment divider and decorator to satisfy both E302 and isort) + E127 x5 (_cost_distance_dask_cupy L461-462, _cost_distance_dask_iterative L1005-1007 signature continuation over-indent). Cat 4 isort: dataset_support before utils, utils from-import reflowed to 100 cols. Cat 2 grep clean. flake8+isort clean on module after fix; 57 cost_distance tests pass (CUDA available). Pre-existing test-file lint (F401 L314/L504, E201 L449, isort drift) left untouched - out of module scope. PR open." focal,2026-05-29,2731,HIGH,3;4;5,"F401 not_implemented_func (import line 36, unused, not re-exported). isort: stdlib reorder (import math before from-imports), dropped stray blank lines in import groups, alphabetised+rewrapped convolution/utils from-imports, moved dataset_support import into order. Cat 5: mutable default excludes=[np.nan] in mean() (line 238) -> None sentinel, resolved to [np.nan] in body; never mutated so behaviour preserved; regression test test_mean_default_excludes_does_not_leak added. Cat 1/2 clean. 115 focal tests pass. PR pending." geotiff,2026-06-12,3259,MEDIUM,4,"Re-sweep after #3082: flake8 baseline 0 across whole subpackage incl tests. Cat 4 only: isort drift in _writers/gpu.py (the from .._attrs import block wrapped at ~80 cols instead of configured 100); rewrapped. Cat 1/2/3/5 clean: no E/W/F codes, no bare except or mutable defaults; type=/id= grep hits are getaddrinfo/pytest.param kwargs and ': list'/': dict' annotation hits, not shadowed builtins. 734 write+gpu tests pass (CUDA available). PR via #3259." hydro-d8,2026-05-29,2705,HIGH,1;3;4,"flake8+isort over the 13 D8 files only (dinf/mfd out of scope). Cat 3 HIGH: F401 x2 (flow_length_d8 function-local _compute_accum_seeds never called; snap_pour_point_d8 module-level cuda_args unused) - both confirmed dead, no re-export. Cat 1: E127/E128 continuation-indent x90 (mostly multi-line def signatures); E302/E303 blank-line cluster in watershed_d8; E501 x4 (flow_path_d8 + snap_pour_point_d8, wrapped ternaries). Cat 4: isort import-block reordering on all 13 files. No Cat 2 (W-codes), no Cat 5 (grep clean: no bare except, mutable defaults, ==None/==True, or shadowed builtins). flake8+isort clean after fix; 385 D8 tests pass. flow_direction_d8 needed manual blank-line placement to satisfy both isort and E302." diff --git a/xrspatial/cost_distance.py b/xrspatial/cost_distance.py index a080b06cd..56cecf857 100644 --- a/xrspatial/cost_distance.py +++ b/xrspatial/cost_distance.py @@ -29,7 +29,6 @@ from __future__ import annotations import math as _math -from functools import partial from math import sqrt import numpy as np @@ -48,18 +47,16 @@ class cupy: # type: ignore[no-redef] ndarray = False -from xrspatial.utils import ( - _dask_task_name_kwargs, - _validate_raster, - cuda_args, get_dataarray_resolution, ngjit, - has_cuda_and_cupy, is_cupy_array, is_dask_cupy, -) from xrspatial.dataset_support import supports_dataset +from xrspatial.utils import (_dask_task_name_kwargs, _validate_raster, cuda_args, + get_dataarray_resolution, has_cuda_and_cupy, is_cupy_array, + is_dask_cupy, ngjit) # --------------------------------------------------------------------------- # Numba binary min-heap (three parallel arrays: keys, rows, cols) # --------------------------------------------------------------------------- + @ngjit def _heap_push(keys, rows, cols, size, key, row, col): """Push (key, row, col) onto the heap. Returns new size.""" @@ -458,8 +455,8 @@ def _cost_distance_cupy(source_data, friction_data, cellsize_x, cellsize_y, def _cost_distance_dask_cupy(source_da, friction_da, - cellsize_x, cellsize_y, max_cost, - target_values, dy, dx, dd): + cellsize_x, cellsize_y, max_cost, + target_values, dy, dx, dd): """Dask+CuPy cost distance. Bounded max_cost: ``da.map_overlap`` with per-chunk GPU relaxation. @@ -1002,9 +999,9 @@ def _process_tile(iy, ix, tile_cache, def _cost_distance_dask_iterative(source_da, friction_da, - cellsize_x, cellsize_y, - max_cost, target_values, - dy, dx, dd): + cellsize_x, cellsize_y, + max_cost, target_values, + dy, dx, dd): """Iterative boundary-only Dijkstra for arbitrarily large dask arrays. Memory usage is O(sqrt(N)) for inter-iteration storage. @@ -1204,7 +1201,7 @@ def cost_distance( friction: xr.DataArray, x: str = "x", y: str = "y", - target_values: list = [], + target_values: list = None, max_cost: float = np.inf, connectivity: int = 8, ) -> xr.DataArray: @@ -1259,6 +1256,8 @@ def cost_distance( cellsize_x = abs(float(cellsize_x)) cellsize_y = abs(float(cellsize_y)) + if target_values is None: + target_values = [] target_values = np.asarray(target_values, dtype=np.float64) max_cost_f = float(max_cost) diff --git a/xrspatial/tests/test_cost_distance.py b/xrspatial/tests/test_cost_distance.py index 309a515e2..62060b350 100644 --- a/xrspatial/tests/test_cost_distance.py +++ b/xrspatial/tests/test_cost_distance.py @@ -861,3 +861,47 @@ def test_cupy_memory_guard_passes_for_small_raster(): out = _compute(result) assert out[0, 0] == 0.0 np.testing.assert_allclose(out[0, 1], 1.0, atol=1e-5) + + +# ----------------------------------------------------------------------- +# target_values default-argument handling +# ----------------------------------------------------------------------- + +def test_default_target_values_does_not_leak(): + """The default target_values must not retain state across calls.""" + source = np.array([ + [1.0, 0.0, 0.0], + [0.0, 0.0, 0.0], + [0.0, 0.0, 0.0], + ]) + friction = np.ones((3, 3)) + + raster = _make_raster(source) + fric = _make_raster(friction) + + first = _compute(cost_distance(raster, fric)) + # A second call with the default must produce the identical result; + # a mutable default that got mutated would diverge here. + second = _compute(cost_distance(raster, fric)) + np.testing.assert_array_equal(first, second) + assert first[0, 0] == 0.0 + + +def test_target_values_none_matches_empty_default(): + """Passing target_values=None behaves like the empty default.""" + source = np.array([ + [0.0, 1.0, 0.0], + [0.0, 0.0, 0.0], + [0.0, 2.0, 0.0], + ]) + friction = np.ones((3, 3)) + + raster = _make_raster(source) + fric = _make_raster(friction) + + default_out = _compute(cost_distance(raster, fric)) + none_out = _compute(cost_distance(raster, fric, target_values=None)) + np.testing.assert_array_equal(default_out, none_out) + # Both value=1 and value=2 pixels are sources under the empty default. + assert none_out[0, 1] == 0.0 + assert none_out[2, 1] == 0.0