Skip to content

Document surface_distance backends, geodesic limits, and add runnable examples - #3724

Open
brendancol wants to merge 2 commits into
mainfrom
deep-sweep-documentation-surface_distance-2026-08-16
Open

Document surface_distance backends, geodesic limits, and add runnable examples#3724
brendancol wants to merge 2 commits into
mainfrom
deep-sweep-documentation-surface_distance-2026-08-16

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3714.

grep -c '>>>' xrspatial/surface_distance.py returned 0. The three public
functions were also silent about which array backends they accept, and about
the fact that method='geodesic' runs on one backend out of four. This PR is
docstrings and tests only; no behaviour changes.

Changes

Backend statement. surface_distance(), surface_allocation(), and
surface_direction() now say they support NumPy, CuPy, Dask with NumPy, and
Dask with CuPy backed DataArrays and return the type they were given. That
matches cost_distance()'s existing wording and what _compute() actually
dispatches to. Verified by running all twelve function/backend combinations
on a CUDA host — every one produced a float32 result, which is what the
Returns sections claim.

Geodesic caveat. The method parameter now says geodesic requires a
NumPy-backed DataArray. Observed:

numpy        surface_distance     OK dtype=float32
cupy         surface_distance     RAISED NotImplementedError: geodesic mode is not yet supported for CuPy arrays
dask+numpy   surface_distance     RAISED NotImplementedError: geodesic mode is not yet supported for Dask arrays
dask+cupy    surface_distance     RAISED NotImplementedError: geodesic mode is not yet supported for Dask+CuPy arrays

Examples. Each function gets a .. sourcecode:: python block. The
surface_distance() one puts a peak in the middle of a flat 3x3 grid so the
output shows the thing the function exists for: the far corner is cheaper to
reach around the peak (3.41) than the summit is to climb (3.32). Every pinned
output came from executing the example; the new tests re-run them.

Known-issue note on surface_direction(). While checking the backend
claims I found that the unbounded dask path returns bearings measured from
the wrong origin in every chunk but the first. That is a code bug, not a
documentation bug, so it is filed separately as #3713 and not fixed here; the
docstring now points at it rather than implying dask output is trustworthy.
The note should come out when #3713 lands, and no test pins it, so removing
it will not turn CI red.

Tests

xrspatial/tests/test_surface_distance.py gains 13 tests in the shape of the
existing test_docstring_states_all_backends /
test_docstring_has_examples_section guards in test_proximity.py,
test_cost_distance.py, and test_edge_detection.py:

  • every public function has an Examples section containing >>>, and no
    three-dot ... sourcecode:: (which renders as literal text)
  • the backend phrases and the geodesic caveat are present
  • geodesic on a dask raster raises NotImplementedError, so the documented
    limitation stays tied to the behaviour
  • the pinned output of all three examples matches what the functions return
$ pytest xrspatial/tests/test_surface_distance.py -q
50 passed

37 before, 50 after. xrspatial/tests/test_accessor.py (98 passed) and
test_dask_task_names.py also stay green.

One pre-existing flake8 F841 at test_surface_distance.py:236 predates this
branch and is left alone.

Found by /sweep-documentation on surface_distance.

…#3714)

The three public functions in surface_distance.py had no runnable example,
no statement of which array backends they support, and no note that
method='geodesic' works on NumPy input only.

- State NumPy / CuPy / Dask+NumPy / Dask+CuPy support, matching the wording
  cost_distance() already uses.
- Say on the method parameter that geodesic requires a NumPy-backed
  DataArray; the other three backends raise NotImplementedError.
- Add a .. sourcecode:: python Examples block to each function, output
  pinned to a real run on this host.
- Note on surface_direction() that the unbounded dask path currently
  returns wrong bearings, and link the fix to #3713.
- Add docstring guard tests plus checks that the pinned example output is
  what the functions return.
@brendancol brendancol added documentation Improvements or additions to documentation area:surface Area: surface labels Aug 16, 2026
@brendancol

Copy link
Copy Markdown
Contributor Author

Self-review

Scope. Two files, docstrings and tests. No kernel, dispatcher, or
finalizer was touched, so no backend behaviour can change. git diff on
xrspatial/surface_distance.py is entirely inside triple-quoted strings.

Things I checked before writing the wording.

  • The backend claim is measured, not assumed. All three functions were run
    on numpy, cupy, dask+numpy, and dask+cupy, bounded and unbounded, and all
    twelve combinations returned float32. _compute() at
    xrspatial/surface_distance.py:1362-1408 is what routes them.
  • The geodesic claim is measured too — the three NotImplementedError
    raises are at :1370, :1378, and :1400, and
    test_geodesic_rejects_dask now holds the docstring to one of them.
  • Cat 4 was already clean: all three functions appear in
    docs/source/reference/proximity.rst:78-92, none duplicated, none stale.
  • numpydoc.docscrape.FunctionDoc parses all three docstrings after the
    change, with Parameters, Returns, Examples, and (for
    surface_direction) Notes all populated.

Deliberate non-changes.

  • surface_allocation() and surface_direction() keep the combined
    x, y, target_values, max_distance, connectivity, method / See :func:surface_distance`` entry. numpydoc parses it as one multi-name
    parameter and renders the cross-reference, so nothing is misleading. It
    would fail a strict test_docstring_params_match_signature port, which
    is why I did not add that particular guard here. Recorded as LOW in the
    sweep state.
  • The surface_direction() dask defect is left unfixed on purpose. The
    documentation sweep is doc-only; fixing the coordinate space in
    _finalize_direction() is a behaviour change and belongs to surface_direction() returns wrong bearings on dask rasters: block-local pixel indices subtracted from global source indices #3713.
  • Pre-existing flake8 F841 at test_surface_distance.py:236 is untouched.

Risk. The pinned example outputs are the only thing that can rot, and
the three new *_matches_output tests catch that on every run rather than
letting the docs drift silently. I used result.values instead of the full
DataArray repr because the repr gained a Size: line in xarray 2025.12 and
would have needed re-pinning on every repr change.

@brendancol

Copy link
Copy Markdown
Contributor Author

Merge-order note for whoever reviews this: the surface_direction docstring here gains a Notes block stating that dask bearings "are currently measured from the wrong origin in every chunk except the first", pointing at #3713.

PR #3729 (test-coverage sweep, same /deep-sweep run) fixes #3713. If #3729 lands first, that paragraph becomes documentation of a bug that no longer exists, and it will keep pointing users at a closed issue.

Suggested order: merge #3729 first, then drop the stale paragraph from this PR before merging. The rest of this PR — the backend-support statement, the method='geodesic' NumPy-only caveat, and the runnable examples — is unaffected either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:surface Area: surface documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

surface_distance/allocation/direction docstrings have no examples, no backend statement, and no geodesic caveat

1 participant