Accept target_values=None in surface_distance trio (#3712) - #3746
Draft
shaikn6 wants to merge 1 commit into
Draft
Conversation
surface_distance, surface_allocation and surface_direction declared target_values: list = [] while proximity, allocation, direction and cost_distance use None and normalize to [] in the body. Passing target_values=None (common when wrapper code threads an optional arg through) reached np.asarray(None) and failed before the numba kernel. Change the three signatures to None and add the sibling 'if target_values is None: target_values = []' guard in the shared _compute dispatcher. [] and None both mean 'every non-zero finite pixel is a source', so callers passing an explicit list are unaffected. Also removes the mutable default argument the siblings already avoid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3712
Proposed Changes
surface_distance,surface_allocation,surface_directionnow accepttarget_values=None(the value wrapper code passes when its own optional arg is unset), matchingproximity/allocation/direction/cost_distance.target_values: list = []totarget_values: list = Noneand added the siblingif target_values is None: target_values = []guard in the shared_computedispatcher.test_target_values_none_matches_empty) parametrized over all three functions.Why / evidence
The proximity trio and
cost_distanceall declaretarget_values: list = Noneand normalize to[]in the body. The threesurface_*functions declaredtarget_values: list = []with noNoneguard, sotarget_values=Nonefell through tonp.asarray(None, dtype=np.float64)(a 0-d array) and blew up before the numba kernel. On numpy 2.3 it surfaces as the existingValueError: target_values must be a 1-D sequence; the issue reports a ~40-line numba TypingError on older numpy. Either way it is broken.[]as a default is also the mutable-default anti-pattern the siblings already avoid.Confirmed with the reproduction from #3712 on this checkout: pre-fix,
surface_distance/allocation/direction(target_values=None)raise while the other four return normally; post-fix all seven succeed.Change
[]andNoneboth mean "treat every non-zero finite pixel as a source", so callers passing an explicit list are unaffected and callers passingNonego from a traceback to working code. No parameter renamed, no accepted input changes meaning, so no deprecation shim is needed.Note:
balanced_allocationcarries the sametarget_values: list = []default (flagged in #3712 as out of scope) and is left untouched here.Verified
flake8 / isort: no new violations introduced by this change (the pre-existing
surface_distance.pyisort drift is tracked separately in #3710).