From 209acf4af4ecf44ec32e95fa5f2173bf555df11c Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 5 Jun 2026 19:21:52 -0700 Subject: [PATCH 1/4] Fix broken hydrology accessor delegations (#2981) The 17 hydrology accessor methods (fill, flow_direction, watershed, basin, twi, hand, etc.) imported from per-algorithm modules that were consolidated into xrspatial/hydro.py, so calling them raised ModuleNotFoundError. Repoint the lazy imports at .hydro. --- xrspatial/accessor.py | 68 +++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/xrspatial/accessor.py b/xrspatial/accessor.py index 05198d850..b0364f0ca 100644 --- a/xrspatial/accessor.py +++ b/xrspatial/accessor.py @@ -352,71 +352,71 @@ def roughness(self, **kwargs): # ---- Hydrology ---- def flow_direction(self, **kwargs): - from .flow_direction import flow_direction + from .hydro import flow_direction return flow_direction(self._obj, **kwargs) def flow_direction_dinf(self, **kwargs): - from .flow_direction_dinf import flow_direction_dinf + from .hydro import flow_direction_dinf return flow_direction_dinf(self._obj, **kwargs) def flow_direction_mfd(self, **kwargs): - from .flow_direction_mfd import flow_direction_mfd + from .hydro import flow_direction_mfd return flow_direction_mfd(self._obj, **kwargs) def flow_accumulation(self, **kwargs): - from .flow_accumulation import flow_accumulation + from .hydro import flow_accumulation return flow_accumulation(self._obj, **kwargs) def flow_accumulation_mfd(self, **kwargs): - from .flow_accumulation_mfd import flow_accumulation_mfd + from .hydro import flow_accumulation_mfd return flow_accumulation_mfd(self._obj, **kwargs) def watershed(self, pour_points, **kwargs): - from .watershed import watershed + from .hydro import watershed return watershed(self._obj, pour_points, **kwargs) def basin(self, **kwargs): - from .basin import basin + from .hydro import basin return basin(self._obj, **kwargs) def basins(self, **kwargs): - from .watershed import basins + from .hydro import basins return basins(self._obj, **kwargs) def sink(self, **kwargs): - from .sink import sink + from .hydro import sink return sink(self._obj, **kwargs) def fill(self, **kwargs): - from .fill import fill + from .hydro import fill return fill(self._obj, **kwargs) def stream_order(self, flow_accum, **kwargs): - from .stream_order import stream_order + from .hydro import stream_order return stream_order(self._obj, flow_accum, **kwargs) def stream_link(self, flow_accum, **kwargs): - from .stream_link import stream_link + from .hydro import stream_link return stream_link(self._obj, flow_accum, **kwargs) def snap_pour_point(self, pour_points, **kwargs): - from .snap_pour_point import snap_pour_point + from .hydro import snap_pour_point return snap_pour_point(self._obj, pour_points, **kwargs) def flow_path(self, start_points, **kwargs): - from .flow_path import flow_path + from .hydro import flow_path return flow_path(self._obj, start_points, **kwargs) def flow_length(self, **kwargs): - from .flow_length import flow_length + from .hydro import flow_length return flow_length(self._obj, **kwargs) def twi(self, slope_agg, **kwargs): - from .twi import twi + from .hydro import twi return twi(self._obj, slope_agg, **kwargs) def hand(self, flow_accum, elevation, **kwargs): - from .hand import hand + from .hydro import hand return hand(self._obj, flow_accum, elevation, **kwargs) # ---- Flood ---- @@ -964,71 +964,71 @@ def roughness(self, **kwargs): # ---- Hydrology ---- def flow_direction(self, **kwargs): - from .flow_direction import flow_direction + from .hydro import flow_direction return flow_direction(self._obj, **kwargs) def flow_direction_dinf(self, **kwargs): - from .flow_direction_dinf import flow_direction_dinf + from .hydro import flow_direction_dinf return flow_direction_dinf(self._obj, **kwargs) def flow_direction_mfd(self, **kwargs): - from .flow_direction_mfd import flow_direction_mfd + from .hydro import flow_direction_mfd return flow_direction_mfd(self._obj, **kwargs) def flow_accumulation(self, **kwargs): - from .flow_accumulation import flow_accumulation + from .hydro import flow_accumulation return flow_accumulation(self._obj, **kwargs) def flow_accumulation_mfd(self, **kwargs): - from .flow_accumulation_mfd import flow_accumulation_mfd + from .hydro import flow_accumulation_mfd return flow_accumulation_mfd(self._obj, **kwargs) def watershed(self, pour_points, **kwargs): - from .watershed import watershed + from .hydro import watershed return watershed(self._obj, pour_points, **kwargs) def basin(self, **kwargs): - from .basin import basin + from .hydro import basin return basin(self._obj, **kwargs) def basins(self, **kwargs): - from .watershed import basins + from .hydro import basins return basins(self._obj, **kwargs) def sink(self, **kwargs): - from .sink import sink + from .hydro import sink return sink(self._obj, **kwargs) def fill(self, **kwargs): - from .fill import fill + from .hydro import fill return fill(self._obj, **kwargs) def stream_order(self, flow_accum, **kwargs): - from .stream_order import stream_order + from .hydro import stream_order return stream_order(self._obj, flow_accum, **kwargs) def stream_link(self, flow_accum, **kwargs): - from .stream_link import stream_link + from .hydro import stream_link return stream_link(self._obj, flow_accum, **kwargs) def snap_pour_point(self, pour_points, **kwargs): - from .snap_pour_point import snap_pour_point + from .hydro import snap_pour_point return snap_pour_point(self._obj, pour_points, **kwargs) def flow_path(self, start_points, **kwargs): - from .flow_path import flow_path + from .hydro import flow_path return flow_path(self._obj, start_points, **kwargs) def flow_length(self, **kwargs): - from .flow_length import flow_length + from .hydro import flow_length return flow_length(self._obj, **kwargs) def twi(self, slope_agg, **kwargs): - from .twi import twi + from .hydro import twi return twi(self._obj, slope_agg, **kwargs) def hand(self, flow_accum, elevation, **kwargs): - from .hand import hand + from .hydro import hand return hand(self._obj, flow_accum, elevation, **kwargs) # ---- Flood ---- From a6154727735761217ef1449e7a8fa1b7b23b8920 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 5 Jun 2026 19:23:11 -0700 Subject: [PATCH 2/4] Surface standalone docstrings on .xrs accessor methods (#2981) Accessor methods were thin delegating wrappers with no docstrings, so help(da.xrs.slope) showed nothing useful. Copy each standalone function's docstring onto its accessor method at import time. Hydrology unified wrappers carry only a stub dispatcher docstring, so their help text is sourced from the documented default-algorithm (*_d8) variants. --- xrspatial/accessor.py | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/xrspatial/accessor.py b/xrspatial/accessor.py index b0364f0ca..54a5fa919 100644 --- a/xrspatial/accessor.py +++ b/xrspatial/accessor.py @@ -1333,3 +1333,60 @@ def open_geotiff(self, source, *, auto_reproject=False, var=None, **kwargs): def rechunk_no_shuffle(self, **kwargs): from .utils import rechunk_no_shuffle return rechunk_no_shuffle(self._obj, **kwargs) + + +# --------------------------------------------------------------------------- +# Surface standalone-function docstrings on accessor methods so that, e.g., +# ``help(da.xrs.slope)`` shows the same documentation as ``help(slope)``. +# --------------------------------------------------------------------------- + +# Accessor method name -> name of the standalone function (in the top-level +# ``xrspatial`` namespace) whose docstring should be surfaced. Only needed +# when the method name differs from the function name, or when the direct +# delegate's docstring is a generic dispatcher: the hydrology unified wrappers +# route by ``routing=`` and carry only a stub docstring, so their help text is +# taken from the documented default-algorithm (``*_d8``) variants instead. +_DOC_SOURCE_OVERRIDES = { + 'focal_mean': 'mean', + 'zonal_hypsometric_integral': 'hypsometric_integral', + 'fill': 'fill_d8', + 'flow_direction': 'flow_direction_d8', + 'flow_accumulation': 'flow_accumulation_d8', + 'basin': 'basin_d8', + 'basins': 'basins_d8', + 'watershed': 'watershed_d8', + 'snap_pour_point': 'snap_pour_point_d8', + 'flow_path': 'flow_path_d8', + 'flow_length': 'flow_length_d8', + 'sink': 'sink_d8', + 'stream_link': 'stream_link_d8', + 'stream_order': 'stream_order_d8', + 'twi': 'twi_d8', + 'hand': 'hand_d8', +} + + +def _delegated_doc(method_name): + """Return the docstring to surface for an accessor method, or None.""" + if method_name == 'min_observable_height': + from .experimental.min_observable_height import min_observable_height + return min_observable_height.__doc__ + import xrspatial + source_name = _DOC_SOURCE_OVERRIDES.get(method_name, method_name) + return getattr(getattr(xrspatial, source_name, None), '__doc__', None) + + +def _attach_delegated_docs(cls): + for name, member in vars(cls).items(): + if name.startswith('_') or not callable(member) or member.__doc__: + continue + try: + doc = _delegated_doc(name) + except Exception: + continue + if doc: + member.__doc__ = doc + + +for _cls in (XrsSpatialDataArrayAccessor, XrsSpatialDatasetAccessor): + _attach_delegated_docs(_cls) From b8c3d072f890a6b7a5d4b86a2e60375987782d36 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 5 Jun 2026 19:25:08 -0700 Subject: [PATCH 3/4] Add accessor docstring and hydrology delegation tests (#2981) Cover help() docstring surfacing (including focal_mean->mean and the hydro *_d8 doc source), a drift guard requiring every public accessor method to carry a useful docstring, and regression tests that the 17 hydrology delegations resolve and match their standalone functions. --- xrspatial/tests/test_accessor.py | 103 +++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/xrspatial/tests/test_accessor.py b/xrspatial/tests/test_accessor.py index a5d669f1f..f8f2d5cb4 100644 --- a/xrspatial/tests/test_accessor.py +++ b/xrspatial/tests/test_accessor.py @@ -396,3 +396,106 @@ def test_ds_plot_without_matplotlib_raises(monkeypatch, elevation): monkeypatch.setitem(sys.modules, 'matplotlib.pyplot', None) with pytest.raises(ImportError, match=r"xarray-spatial\[plot\]"): ds.xrs.plot() + + +# --------------------------------------------------------------------------- +# 9. help() — standalone docstrings surfaced on accessor methods (#2981) +# --------------------------------------------------------------------------- + +import inspect # noqa: E402 + +from xrspatial.accessor import ( # noqa: E402 + XrsSpatialDataArrayAccessor, + XrsSpatialDatasetAccessor, +) + +# The hydrology unified wrappers carry only this generic dispatcher stub; the +# accessor must surface the documented *_d8 variant docs instead. +_GENERIC_HYDRO_DOC = 'Map routing algorithm names' + + +@pytest.mark.parametrize('method_name, source', [ + ('slope', 'slope'), + ('aspect', 'aspect'), + ('hillshade', 'hillshade'), + ('curvature', 'curvature'), + ('focal_mean', 'mean'), # method name differs from function + ('ndvi', 'ndvi'), + ('fill', 'fill_d8'), # hydro wrapper -> documented d8 variant + ('watershed', 'watershed_d8'), +]) +def test_accessor_docstring_matches_source(method_name, source): + func = getattr(xrspatial, source) + method = getattr(XrsSpatialDataArrayAccessor, method_name) + assert inspect.getdoc(method), f'{method_name} has no docstring' + assert inspect.getdoc(method) == inspect.getdoc(func) + + +def test_help_text_surfaces_on_instance(elevation): + """help(da.xrs.slope) sees the same docstring as help(slope).""" + from xrspatial import slope + assert inspect.getdoc(elevation.xrs.slope) == inspect.getdoc(slope) + + +def test_handwritten_method_docs_preserved(): + """Methods with their own docstrings are not overwritten.""" + assert XrsSpatialDataArrayAccessor.plot.__doc__.lstrip().startswith( + 'Plot the DataArray' + ) + assert XrsSpatialDataArrayAccessor.to_geotiff.__doc__.lstrip().startswith( + 'Write this DataArray' + ) + + +@pytest.mark.parametrize( + 'cls', [XrsSpatialDataArrayAccessor, XrsSpatialDatasetAccessor] +) +def test_every_public_method_documented(cls): + """Drift guard: every public accessor method has a useful docstring.""" + undocumented = [] + for name, member in vars(cls).items(): + if name.startswith('_') or not callable(member): + continue + doc = (member.__doc__ or '').strip() + if not doc or doc.startswith(_GENERIC_HYDRO_DOC): + undocumented.append(name) + assert not undocumented, f'methods lacking a useful docstring: {undocumented}' + + +# --------------------------------------------------------------------------- +# 10. Hydrology accessor methods import and run (regression for #2981) +# --------------------------------------------------------------------------- + +@pytest.mark.parametrize('method_name', [ + 'fill', 'flow_direction', 'flow_accumulation', 'sink', 'basin', 'flow_length', +]) +def test_hydro_accessor_matches_direct(method_name, elevation): + """Single-input hydro accessor methods match the standalone function.""" + func = getattr(xrspatial, method_name) + expected = func(elevation) + result = getattr(elevation.xrs, method_name)() + xr.testing.assert_identical(result, expected) + + +_ALL_HYDRO_METHODS = [ + 'flow_direction', 'flow_direction_dinf', 'flow_direction_mfd', + 'flow_accumulation', 'flow_accumulation_mfd', 'watershed', 'basin', 'basins', + 'sink', 'fill', 'stream_order', 'stream_link', 'snap_pour_point', + 'flow_path', 'flow_length', 'twi', 'hand', +] + + +@pytest.mark.parametrize('method_name', _ALL_HYDRO_METHODS) +def test_hydro_accessor_delegation_resolves(method_name, elevation): + """The lazy import no longer points at a removed per-algorithm module. + + Methods needing extra positional args raise TypeError (or another + runtime error) once the import succeeds; only a ModuleNotFoundError + means the delegation is still broken. + """ + try: + getattr(elevation.xrs, method_name)() + except ModuleNotFoundError as exc: # pragma: no cover - the bug we fixed + pytest.fail(f'{method_name} delegation still broken: {exc}') + except Exception: + pass # any non-import error proves the import path resolved From 01a53485cdde94cc33461724cdaee3c21fbedd67 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Fri, 5 Jun 2026 19:28:28 -0700 Subject: [PATCH 4/4] Address review: harden doc lookup, tidy imports (#2981) - _delegated_doc: resolve the source function before reading __doc__ so a missing source name yields None instead of None.__doc__ ('The type of the None singleton.') being attached as a bogus docstring. - Replace the module-level attach loop with two explicit calls (no leaked _cls binding). - Move the test module's inspect/accessor imports to the top of the file. --- xrspatial/accessor.py | 7 ++++--- xrspatial/tests/test_accessor.py | 13 ++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/xrspatial/accessor.py b/xrspatial/accessor.py index 54a5fa919..370352552 100644 --- a/xrspatial/accessor.py +++ b/xrspatial/accessor.py @@ -1373,7 +1373,8 @@ def _delegated_doc(method_name): return min_observable_height.__doc__ import xrspatial source_name = _DOC_SOURCE_OVERRIDES.get(method_name, method_name) - return getattr(getattr(xrspatial, source_name, None), '__doc__', None) + func = getattr(xrspatial, source_name, None) + return func.__doc__ if func is not None else None def _attach_delegated_docs(cls): @@ -1388,5 +1389,5 @@ def _attach_delegated_docs(cls): member.__doc__ = doc -for _cls in (XrsSpatialDataArrayAccessor, XrsSpatialDatasetAccessor): - _attach_delegated_docs(_cls) +_attach_delegated_docs(XrsSpatialDataArrayAccessor) +_attach_delegated_docs(XrsSpatialDatasetAccessor) diff --git a/xrspatial/tests/test_accessor.py b/xrspatial/tests/test_accessor.py index f8f2d5cb4..315e83033 100644 --- a/xrspatial/tests/test_accessor.py +++ b/xrspatial/tests/test_accessor.py @@ -1,10 +1,16 @@ """Tests for the .xrs xarray accessors.""" +import inspect + import numpy as np import pytest import xarray as xr import xrspatial # noqa: F401 — triggers accessor registration +from xrspatial.accessor import ( + XrsSpatialDataArrayAccessor, + XrsSpatialDatasetAccessor, +) # --------------------------------------------------------------------------- @@ -402,13 +408,6 @@ def test_ds_plot_without_matplotlib_raises(monkeypatch, elevation): # 9. help() — standalone docstrings surfaced on accessor methods (#2981) # --------------------------------------------------------------------------- -import inspect # noqa: E402 - -from xrspatial.accessor import ( # noqa: E402 - XrsSpatialDataArrayAccessor, - XrsSpatialDatasetAccessor, -) - # The hydrology unified wrappers carry only this generic dispatcher stub; the # accessor must surface the documented *_d8 variant docs instead. _GENERIC_HYDRO_DOC = 'Map routing algorithm names'