surface_distance trio rejects target_values=None while proximity, allocation, direction and cost_distance accept it

Open Beginner friendly
#3,712 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
82/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
numpy, python
Domain
data

Research direction

Start with xrspatial/surface_distance.py at the surface_distance, surface_allocation, and surface_direction entry points around lines 1424-1527, then compare their target_values handling with the sibling functions listed in xrspatial/proximity.py and xrspatial/cost_distance.py. Run the reproduction against all seven functions; done means each surface function accepts target_values=None without a numba TypingError and explicit target lists still work.

Written by the indexing model from the issue text.

Description

api area:surface bug severity:medium sweep-api-consistency

surface_distance, surface_allocation and surface_direction declare target_values: list = []. Every other function in the same family declares target_values: list = None and normalizes it to [] in the body:

function file default
proximity xrspatial/proximity.py:1607 None
allocation xrspatial/proximity.py:1767 None
direction xrspatial/proximity.py:1928 None
cost_distance xrspatial/cost_distance.py:1219 None
surface_distance xrspatial/surface_distance.py:1424 []
surface_allocation xrspatial/surface_distance.py:1485 []
surface_direction xrspatial/surface_distance.py:1527 []

Two consequences.

The first is that passing target_values=None explodes. The proximity trio and cost_distance accept it because they do if target_values is None: target_values = []. surface_distance has no such guard, so None reaches np.asarray(None, dtype=np.float64), which produces a 0-d array, which is then handed to the _seed_sources numba kernel and dies in type inference. The traceback runs about 40 lines about getitem(array(float64, 0d, C), int64) and never mentions target_values.

That is not a contrived call. Wrapper code routinely threads target_values through from its own optional argument, which is None when the caller did not set it. The pattern works against four functions in this family and fails against three.

The second is that [] as a default is a mutable default argument. Nothing mutates it today, since _compute immediately does np.asarray, so there is no live aliasing bug. It is still the anti-pattern the sibling functions already avoid.

Reproduction

import numpy as np
import xarray as xr
from xrspatial import (proximity, allocation, direction,
                       surface_distance, surface_allocation, surface_direction)
from xrspatial.cost_distance import cost_distance

H = W = 8
src = np.zeros((H, W)); src[0, 0] = 1.0
elev = np.zeros((H, W))
coords = {'y': np.arange(H)[::-1].astype(float), 'x': np.arange(W).astype(float)}
raster = xr.DataArray(src, dims=['y', 'x'], coords=coords)
elevation = xr.DataArray(elev, dims=['y', 'x'], coords=coords)

for name, fn, args in [
    ("proximity", proximity, (raster,)),
    ("allocation", allocation, (raster,)),
    ("direction", direction, (raster,)),
    ("cost_distance", cost_distance, (raster, elevation)),
    ("surface_distance", surface_distance, (raster, elevation)),
    ("surface_allocation", surface_allocation, (raster, elevation)),
    ("surface_direction", surface_direction, (raster, elevation)),
]:
    try:
        fn(*args, target_values=None)
        print(f"{name}(target_values=None) -> OK")
    except Exception as e:
        print(f"{name}(target_values=None) -> {type(e).__name__}")

Observed on this host (Python 3.14.2, numba 0.65.0, numpy 2.3.2):

proximity(target_values=None) -> OK
allocation(target_values=None) -> OK
direction(target_values=None) -> OK
cost_distance(target_values=None) -> OK
surface_distance(target_values=None) -> TypingError
surface_allocation(target_values=None) -> TypingError
surface_direction(target_values=None) -> TypingError

The TypingError body:

No implementation of function Function(<built-in function getitem>) found for signature:
 >>> getitem(array(float64, 0d, C), int64)
...
File "xrspatial/surface_distance.py", line 193:
                for k in range(n_values):
                    if val == target_values[k]:
                    ^

Proposed fix

Change the three signatures to target_values: list = None and add the same if target_values is None: target_values = [] normalization the siblings use. This is not a breaking change: [] and None both mean "treat every non-zero finite pixel as a source", callers passing an explicit list are unaffected, and callers passing None go from a numba traceback to working code. No deprecation shim is needed because no parameter is renamed and no accepted input changes meaning.

Notes

Found by /sweep-api-consistency on surface_distance (category 1 and 4, MEDIUM).

balanced_allocation (xrspatial/balanced_allocation.py) carries the same target_values: list = [] default. Out of scope for this issue, but worth a separate look to see whether it fails the same way.

Dominant language
Python
Stars
972
Forks
92
Avg merge
2d 12h
Merged PRs (30d)
7

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from xarray-contrib/xarray-spatial

All issues in xarray-contrib/xarray-spatial

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.