Skip to content

Commit 04f7b70

Browse files
thodson-usgsclaude
andcommitted
fix(waterdata.xarray): accurate stats docstring; pin wrapper->_fetch routing
Addresses two review gaps: - The shared wrapper docstring promised "a CF-conventions Dataset with series metadata populated", which is false for the stats wrappers -- _build_stats emits a flat, preliminary Dataset with only dataset-level provenance. Parametrize _xr_doc(cf_metadata=...) so get_stats_por/get_stats_date_range describe their actual output. - The include_hash strip was only tested on _fetch in isolation; nothing pinned the wrappers to route through it. Add a test that monkeypatches _fetch and asserts every public wrapper delegates to it (and forwards include_hash for _fetch to drop), guarding against a wrapper reverting to a direct getter call. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent fcfaf14 commit 04f7b70

2 files changed

Lines changed: 39 additions & 7 deletions

File tree

dataretrieval/waterdata/xarray.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -340,14 +340,25 @@ def _fetch(func, args, kwargs):
340340
return func(*args, **kwargs)
341341

342342

343-
def _xr_doc(func):
344-
"""Prepend an xarray note to the wrapped getter's docstring."""
343+
def _xr_doc(func, *, cf_metadata=True):
344+
"""Prepend an xarray note to the wrapped getter's docstring.
345+
346+
``cf_metadata=False`` describes the preliminary stats path, which emits a
347+
flat Dataset without per-variable CF attributes.
348+
"""
349+
returns = (
350+
"a CF-conventions ``xarray.Dataset`` with series metadata populated"
351+
if cf_metadata
352+
else (
353+
"a preliminary, flat ``xarray.Dataset`` (dataset-level provenance "
354+
"only; per-variable CF metadata is not yet populated)"
355+
)
356+
)
345357
note = (
346358
" xarray wrapper: same arguments as "
347-
f"``dataretrieval.waterdata.{func.__name__}``, but returns a\n"
348-
" CF-conventions ``xarray.Dataset`` with series metadata populated.\n"
349-
" Hash-valued ID columns are always omitted here; the\n"
350-
" ``include_hash`` flag does not apply.\n\n"
359+
f"``dataretrieval.waterdata.{func.__name__}``, but returns\n"
360+
f" {returns}. Hash-valued ID columns are always omitted here;\n"
361+
" the ``include_hash`` flag does not apply.\n\n"
351362
)
352363
return note + (func.__doc__ or "")
353364

@@ -391,7 +402,7 @@ def wrapper(*args, **kwargs):
391402
df, base_meta = _fetch(func, args, kwargs)
392403
return _build_stats(df, base_meta, service)
393404

394-
wrapper.__doc__ = _xr_doc(func)
405+
wrapper.__doc__ = _xr_doc(func, cf_metadata=False)
395406
return wrapper
396407

397408

tests/waterdata_xarray_test.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,24 @@ def fake_getter(*args, **kwargs):
195195
assert (df, meta) == ("df", "meta")
196196
assert "include_hash" not in captured
197197
assert captured == {"parameter_code": "00060"}
198+
199+
200+
def test_every_wrapper_routes_through_fetch(monkeypatch):
201+
# Pin the wiring: each public wrapper must delegate the underlying fetch to
202+
# _fetch (which is what strips include_hash). Guards against a wrapper
203+
# quietly reverting to calling the getter directly and leaking the flag.
204+
seen = []
205+
206+
def spy(func, args, kwargs):
207+
seen.append(dict(kwargs))
208+
return pd.DataFrame(), SimpleNamespace(url=None) # empty -> no network
209+
210+
monkeypatch.setattr(wdx, "_fetch", spy)
211+
for name in wdx.__all__:
212+
seen.clear()
213+
ds = getattr(wdx, name)(monitoring_location_id="USGS-1", include_hash=True)
214+
assert isinstance(ds, xr.Dataset)
215+
assert len(seen) == 1, f"{name} did not route through _fetch"
216+
# the wrapper hands include_hash to _fetch; _fetch is what drops it
217+
# (asserted in test_fetch_strips_include_hash)
218+
assert seen[0].get("include_hash") is True

0 commit comments

Comments
 (0)