From 5103dc15c84f19c1c373a7b453634c28fbd75f15 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 15 Jun 2026 06:45:09 -0700 Subject: [PATCH 1/2] Replace mutable default target_values=[] with None sentinel in cost_distance (#3340) cost_distance() declared target_values: list = [], a mutable default shared across calls. The value is rebound via np.asarray on entry so there was no active mutation bug, but the declaration is the unsafe form and drifts from proximity()/allocation(), which use the None sentinel. Switch to target_values: list = None with an internal 'if target_values is None: target_values = []' substitution. Non-breaking (callers passing a list are unaffected; default behavior unchanged), no deprecation shim needed. Adds regression tests pinning the None default and confirming None / [] / omitted are equivalent. --- xrspatial/cost_distance.py | 5 +++- xrspatial/tests/test_cost_distance.py | 42 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/xrspatial/cost_distance.py b/xrspatial/cost_distance.py index a080b06cd..0701dbdb8 100644 --- a/xrspatial/cost_distance.py +++ b/xrspatial/cost_distance.py @@ -1204,7 +1204,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: @@ -1255,6 +1255,9 @@ def cost_distance( if connectivity not in (4, 8): raise ValueError("connectivity must be 4 or 8") + if target_values is None: + target_values = [] + cellsize_x, cellsize_y = get_dataarray_resolution(raster) cellsize_x = abs(float(cellsize_x)) cellsize_y = abs(float(cellsize_y)) diff --git a/xrspatial/tests/test_cost_distance.py b/xrspatial/tests/test_cost_distance.py index 309a515e2..ef6d8a259 100644 --- a/xrspatial/tests/test_cost_distance.py +++ b/xrspatial/tests/test_cost_distance.py @@ -304,6 +304,48 @@ def test_target_values(backend): np.testing.assert_allclose(out[0, 1], 2.0, atol=1e-5) +def test_target_values_default_is_none_sentinel(): + """target_values default is the None sentinel, not a shared mutable list. + + Regression for issue #3340: a mutable default (``list = []``) shares one + list object across calls. cost_distance uses the ``None`` sentinel like + proximity()/allocation() do. + """ + import inspect + + default = inspect.signature(cost_distance).parameters['target_values'].default + assert default is None + + +def test_target_values_none_matches_empty_list(): + """Omitting target_values, passing None, and passing [] are equivalent. + + All three mean "every non-zero finite pixel is a source". + """ + source = np.array([ + [7.0, 0.0, 0.0], + [0.0, 0.0, 0.0], + [0.0, 0.0, 1.0], + ]) + friction_data = np.ones((3, 3)) + raster = _make_raster(source) + friction = _make_raster(friction_data) + + out_omitted = _compute(cost_distance(raster, friction)) + out_none = _compute(cost_distance(raster, friction, target_values=None)) + out_empty = _compute(cost_distance(raster, friction, target_values=[])) + + np.testing.assert_array_equal( + np.nan_to_num(out_omitted), np.nan_to_num(out_none) + ) + np.testing.assert_array_equal( + np.nan_to_num(out_omitted), np.nan_to_num(out_empty) + ) + # Both non-zero finite pixels are sources (cost 0). + assert out_omitted[0, 0] == 0.0 + assert out_omitted[2, 2] == 0.0 + + # ----------------------------------------------------------------------- # Lazy coordinate arrays for dask input # ----------------------------------------------------------------------- From 0c329038a5fd968b41b5d5f670936280fca1d78c Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Mon, 15 Jun 2026 06:46:56 -0700 Subject: [PATCH 2/2] Address review nit: clarify target_values docstring for None default (#3340) --- xrspatial/cost_distance.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xrspatial/cost_distance.py b/xrspatial/cost_distance.py index 0701dbdb8..f9d06febb 100644 --- a/xrspatial/cost_distance.py +++ b/xrspatial/cost_distance.py @@ -1229,7 +1229,7 @@ def cost_distance( Name of the y coordinate. target_values : list, optional Specific pixel values in *raster* to treat as sources. - If empty, all non-zero finite pixels are sources. + If not provided, all non-zero finite pixels are sources. max_cost : float, default=np.inf Maximum accumulated cost. Pixels whose least-cost path exceeds this budget are set to NaN. A finite value enables efficient