Skip to content

Commit 06bf0d0

Browse files
thodson-usgsclaude
andauthored
fix(wqp): repair WQP_Metadata.site_info (was always None) (#305)
WQP_Metadata.site_info never returned anything. Two compounding bugs: 1. All nine getters built WQP_Metadata(response) with no parameters, so self._parameters was always {} and the property always hit the `else: return None` branch -- even for a get_results(siteid=...) query. (PR #295 fixed the property's *location* -- it had been a discarded local function inside __init__ -- but the call sites still passed no params, so the feature stayed dead.) NWIS does it right: NWIS_Metadata(response, **kwargs). 2. The property keyed on `sites`/`site`/`site_no` and called what_sites(sites=...), but WQP's site identifier parameter is `siteid`, so it could never have matched a real WQP query even with params threaded in. Fixes: - All getters now pass the query kwargs: WQP_Metadata(response, **kwargs). - site_info keys on `siteid` and calls what_sites(siteid=...). - Corrected the class docstring (comment not comments; WQP_Metadata not NWIS_Metadata; siteid not sites). - Updated the routing test to the correct `siteid` key and added a regression assertion that get_results threads siteid into the metadata. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent adb6629 commit 06bf0d0

2 files changed

Lines changed: 27 additions & 26 deletions

File tree

dataretrieval/wqp.py

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ def get_results(
155155

156156
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
157157
df = _attach_datetime_columns(df)
158-
return df, WQP_Metadata(response)
158+
return df, WQP_Metadata(response, **kwargs)
159159

160160

161161
def what_sites(
@@ -210,7 +210,7 @@ def what_sites(
210210

211211
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
212212

213-
return df, WQP_Metadata(response)
213+
return df, WQP_Metadata(response, **kwargs)
214214

215215

216216
def what_organizations(
@@ -261,7 +261,7 @@ def what_organizations(
261261

262262
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
263263

264-
return df, WQP_Metadata(response)
264+
return df, WQP_Metadata(response, **kwargs)
265265

266266

267267
def what_projects(ssl_check=True, legacy=True, **kwargs):
@@ -308,7 +308,7 @@ def what_projects(ssl_check=True, legacy=True, **kwargs):
308308

309309
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
310310

311-
return df, WQP_Metadata(response)
311+
return df, WQP_Metadata(response, **kwargs)
312312

313313

314314
def what_activities(
@@ -372,7 +372,7 @@ def what_activities(
372372

373373
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
374374

375-
return df, WQP_Metadata(response)
375+
return df, WQP_Metadata(response, **kwargs)
376376

377377

378378
def what_detection_limits(
@@ -430,7 +430,7 @@ def what_detection_limits(
430430

431431
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
432432

433-
return df, WQP_Metadata(response)
433+
return df, WQP_Metadata(response, **kwargs)
434434

435435

436436
def what_habitat_metrics(
@@ -481,7 +481,7 @@ def what_habitat_metrics(
481481

482482
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
483483

484-
return df, WQP_Metadata(response)
484+
return df, WQP_Metadata(response, **kwargs)
485485

486486

487487
def what_project_weights(ssl_check=True, legacy=True, **kwargs):
@@ -533,7 +533,7 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs):
533533

534534
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
535535

536-
return df, WQP_Metadata(response)
536+
return df, WQP_Metadata(response, **kwargs)
537537

538538

539539
def what_activity_metrics(ssl_check=True, legacy=True, **kwargs):
@@ -585,7 +585,7 @@ def what_activity_metrics(ssl_check=True, legacy=True, **kwargs):
585585

586586
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
587587

588-
return df, WQP_Metadata(response)
588+
return df, WQP_Metadata(response, **kwargs)
589589

590590

591591
def wqp_url(service):
@@ -627,10 +627,10 @@ class WQP_Metadata(BaseMetadata):
627627
Response elapsed time
628628
header : httpx.Headers
629629
Response headers
630-
comments : None
631-
Metadata comments. WQP does not return comments.
632-
site_info : tuple[pd.DataFrame, NWIS_Metadata] | None
633-
Site information if the query included `sites`, `site` or `site_no`.
630+
comment : None
631+
WQP does not return comments.
632+
site_info : tuple[pd.DataFrame, WQP_Metadata] | None
633+
Site information (via ``what_sites``) if the query included a ``siteid``.
634634
"""
635635

636636
def __init__(self, response, **parameters) -> None:
@@ -660,24 +660,21 @@ def __init__(self, response, **parameters) -> None:
660660
def site_info(self) -> tuple[DataFrame, WQP_Metadata] | None:
661661
"""Site information for the query.
662662
663-
Populated when the query included ``sites``, ``site`` or
664-
``site_no`` (in that order of preference); ``None`` otherwise.
663+
Populated (via :func:`dataretrieval.wqp.what_sites`) when the query
664+
included a ``siteid`` (the WQP site identifier, e.g.
665+
``"USGS-05586100"``); ``None`` otherwise.
665666
666667
Returns
667668
-------
668669
df : ``pandas.DataFrame``
669-
Formatted requested data from calling ``wqp.what_sites``.
670+
Site data returned by ``wqp.what_sites``.
670671
md : :obj:`dataretrieval.wqp.WQP_Metadata`
671672
A WQP_Metadata object.
672673
"""
673-
if "sites" in self._parameters:
674-
return what_sites(sites=self._parameters["sites"])
675-
elif "site" in self._parameters:
676-
return what_sites(sites=self._parameters["site"])
677-
elif "site_no" in self._parameters:
678-
return what_sites(sites=self._parameters["site_no"])
679-
else:
674+
siteid = self._parameters.get("siteid")
675+
if siteid is None:
680676
return None
677+
return what_sites(siteid=siteid)
681678

682679

683680
def _check_kwargs(kwargs):

tests/wqp_test.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ def test_get_results(httpx_mock):
4141
assert md.header.get("mock_header") == "value"
4242
assert md.comment is None
4343
assert df["ActivityStartDateTime"].notna().all()
44+
# Regression: the getter must thread the query kwargs into the metadata
45+
# (it previously built WQP_Metadata(response), dropping them), so that
46+
# md.site_info has a siteid to look up instead of always returning None.
47+
assert md._parameters.get("siteid") == "WIDNR_WQX-10032762"
4448

4549

4650
def test_get_results_WQX3(httpx_mock):
@@ -269,7 +273,7 @@ def test_wqp_metadata_site_info_is_accessible_property():
269273

270274

271275
def test_wqp_metadata_site_info_routes_to_what_sites(monkeypatch):
272-
"""When the query carried ``sites`` (or ``site``/``site_no``),
276+
"""When the query carried a ``siteid`` (WQP's site identifier),
273277
``site_info`` delegates to ``wqp.what_sites`` with that identifier."""
274278
import dataretrieval.wqp as wqp_mod
275279

@@ -280,5 +284,5 @@ def fake_what_sites(**kwargs):
280284
return "SENTINEL"
281285

282286
monkeypatch.setattr(wqp_mod, "what_sites", fake_what_sites)
283-
assert _wqp_metadata(sites="USGS-05427718").site_info == "SENTINEL"
284-
assert captured == {"sites": "USGS-05427718"}
287+
assert _wqp_metadata(siteid="USGS-05427718").site_info == "SENTINEL"
288+
assert captured == {"siteid": "USGS-05427718"}

0 commit comments

Comments
 (0)