From c72e785f2100e04d14c933d81f1f9a2d97f3b07b Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Sat, 30 May 2026 14:36:27 -0400 Subject: [PATCH] fix(wqp): repair WQP_Metadata.site_info (was always None) 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) --- dataretrieval/wqp.py | 43 ++++++++++++++++++++----------------------- tests/wqp_test.py | 10 +++++++--- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/dataretrieval/wqp.py b/dataretrieval/wqp.py index e41235a9..0e0adc8d 100644 --- a/dataretrieval/wqp.py +++ b/dataretrieval/wqp.py @@ -155,7 +155,7 @@ def get_results( df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) df = _attach_datetime_columns(df) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_sites( @@ -210,7 +210,7 @@ def what_sites( df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_organizations( @@ -261,7 +261,7 @@ def what_organizations( df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_projects(ssl_check=True, legacy=True, **kwargs): @@ -308,7 +308,7 @@ def what_projects(ssl_check=True, legacy=True, **kwargs): df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_activities( @@ -372,7 +372,7 @@ def what_activities( df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_detection_limits( @@ -430,7 +430,7 @@ def what_detection_limits( df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_habitat_metrics( @@ -481,7 +481,7 @@ def what_habitat_metrics( df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_project_weights(ssl_check=True, legacy=True, **kwargs): @@ -533,7 +533,7 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs): df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def what_activity_metrics(ssl_check=True, legacy=True, **kwargs): @@ -585,7 +585,7 @@ def what_activity_metrics(ssl_check=True, legacy=True, **kwargs): df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False) - return df, WQP_Metadata(response) + return df, WQP_Metadata(response, **kwargs) def wqp_url(service): @@ -627,10 +627,10 @@ class WQP_Metadata(BaseMetadata): Response elapsed time header : httpx.Headers Response headers - comments : None - Metadata comments. WQP does not return comments. - site_info : tuple[pd.DataFrame, NWIS_Metadata] | None - Site information if the query included `sites`, `site` or `site_no`. + comment : None + WQP does not return comments. + site_info : tuple[pd.DataFrame, WQP_Metadata] | None + Site information (via ``what_sites``) if the query included a ``siteid``. """ def __init__(self, response, **parameters) -> None: @@ -660,24 +660,21 @@ def __init__(self, response, **parameters) -> None: def site_info(self) -> tuple[DataFrame, WQP_Metadata] | None: """Site information for the query. - Populated when the query included ``sites``, ``site`` or - ``site_no`` (in that order of preference); ``None`` otherwise. + Populated (via :func:`dataretrieval.wqp.what_sites`) when the query + included a ``siteid`` (the WQP site identifier, e.g. + ``"USGS-05586100"``); ``None`` otherwise. Returns ------- df : ``pandas.DataFrame`` - Formatted requested data from calling ``wqp.what_sites``. + Site data returned by ``wqp.what_sites``. md : :obj:`dataretrieval.wqp.WQP_Metadata` A WQP_Metadata object. """ - if "sites" in self._parameters: - return what_sites(sites=self._parameters["sites"]) - elif "site" in self._parameters: - return what_sites(sites=self._parameters["site"]) - elif "site_no" in self._parameters: - return what_sites(sites=self._parameters["site_no"]) - else: + siteid = self._parameters.get("siteid") + if siteid is None: return None + return what_sites(siteid=siteid) def _check_kwargs(kwargs): diff --git a/tests/wqp_test.py b/tests/wqp_test.py index 5ea4edea..83b69ff4 100644 --- a/tests/wqp_test.py +++ b/tests/wqp_test.py @@ -41,6 +41,10 @@ def test_get_results(httpx_mock): assert md.header.get("mock_header") == "value" assert md.comment is None assert df["ActivityStartDateTime"].notna().all() + # Regression: the getter must thread the query kwargs into the metadata + # (it previously built WQP_Metadata(response), dropping them), so that + # md.site_info has a siteid to look up instead of always returning None. + assert md._parameters.get("siteid") == "WIDNR_WQX-10032762" def test_get_results_WQX3(httpx_mock): @@ -269,7 +273,7 @@ def test_wqp_metadata_site_info_is_accessible_property(): def test_wqp_metadata_site_info_routes_to_what_sites(monkeypatch): - """When the query carried ``sites`` (or ``site``/``site_no``), + """When the query carried a ``siteid`` (WQP's site identifier), ``site_info`` delegates to ``wqp.what_sites`` with that identifier.""" import dataretrieval.wqp as wqp_mod @@ -280,5 +284,5 @@ def fake_what_sites(**kwargs): return "SENTINEL" monkeypatch.setattr(wqp_mod, "what_sites", fake_what_sites) - assert _wqp_metadata(sites="USGS-05427718").site_info == "SENTINEL" - assert captured == {"sites": "USGS-05427718"} + assert _wqp_metadata(siteid="USGS-05427718").site_info == "SENTINEL" + assert captured == {"siteid": "USGS-05427718"}