Skip to content

Commit 9a982c0

Browse files
thodson-usgsclaude
andcommitted
Fix WQP_Metadata.site_info: bind as a real property and forward kwargs
`site_info` was defined inside `WQP_Metadata.__init__` as a nested `@property`-decorated function that was never bound to the class. The decorator returned a property descriptor that was immediately discarded when `__init__` returned, so `md.site_info` always fell through to `BaseMetadata.site_info` and raised `NotImplementedError`. The closure also used `parameters[...]` (the captured `**parameters` kwargs) for the lookup but `self._parameters` for the `in` check — inconsistent. Compounding the issue, every helper called `WQP_Metadata(response)` with no kwargs, so `self._parameters` was always `{}` even after fixing the closure. Move `site_info` to a real property on the class, look up `siteid` first (WQP-native; the previous `sites`/`site`/`site_no` keys reflected an NWIS copy-paste and never matched a WQP query), and thread `**kwargs` through `WQP_Metadata(response, **kwargs)` from all nine call sites so the property has the parameters it needs. Also corrected the docstring: the attribute is `comment` (not `comments`, inherited from `BaseMetadata`), and the `query_time` type is `datetime.timedelta` (not `datetme.timedelta`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 51ac674 commit 9a982c0

2 files changed

Lines changed: 62 additions & 22 deletions

File tree

dataretrieval/wqp.py

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ def get_results(
155155
response = query(url, kwargs, delimiter=";", ssl_check=ssl_check)
156156

157157
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
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(
@@ -265,7 +265,7 @@ def what_organizations(
265265

266266
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
267267

268-
return df, WQP_Metadata(response)
268+
return df, WQP_Metadata(response, **kwargs)
269269

270270

271271
def what_projects(ssl_check=True, legacy=True, **kwargs):
@@ -316,7 +316,7 @@ def what_projects(ssl_check=True, legacy=True, **kwargs):
316316

317317
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
318318

319-
return df, WQP_Metadata(response)
319+
return df, WQP_Metadata(response, **kwargs)
320320

321321

322322
def what_activities(
@@ -380,7 +380,7 @@ def what_activities(
380380

381381
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
382382

383-
return df, WQP_Metadata(response)
383+
return df, WQP_Metadata(response, **kwargs)
384384

385385

386386
def what_detection_limits(
@@ -442,7 +442,7 @@ def what_detection_limits(
442442

443443
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
444444

445-
return df, WQP_Metadata(response)
445+
return df, WQP_Metadata(response, **kwargs)
446446

447447

448448
def what_habitat_metrics(
@@ -497,7 +497,7 @@ def what_habitat_metrics(
497497

498498
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
499499

500-
return df, WQP_Metadata(response)
500+
return df, WQP_Metadata(response, **kwargs)
501501

502502

503503
def what_project_weights(ssl_check=True, legacy=True, **kwargs):
@@ -553,7 +553,7 @@ def what_project_weights(ssl_check=True, legacy=True, **kwargs):
553553

554554
df = pd.read_csv(StringIO(response.text), delimiter=",", low_memory=False)
555555

556-
return df, WQP_Metadata(response)
556+
return df, WQP_Metadata(response, **kwargs)
557557

558558

559559
def what_activity_metrics(ssl_check=True, legacy=True, **kwargs):
@@ -609,7 +609,7 @@ def what_activity_metrics(ssl_check=True, legacy=True, **kwargs):
609609

610610
df = pd.read_csv(StringIO(response.text), delimiter=",")
611611

612-
return df, WQP_Metadata(response)
612+
return df, WQP_Metadata(response, **kwargs)
613613

614614

615615
def wqp_url(service):
@@ -649,14 +649,15 @@ class WQP_Metadata(BaseMetadata):
649649
----------
650650
url : str
651651
Response url
652-
query_time : datetme.timedelta
652+
query_time : datetime.timedelta
653653
Response elapsed time
654654
header : requests.structures.CaseInsensitiveDict
655655
Response headers
656-
comments : None
657-
Metadata comments. WQP does not return comments.
658-
site_info : tuple[pd.DataFrame, NWIS_Metadata] | None
659-
Site information if the query included `sites`, `site` or `site_no`.
656+
comment : None
657+
Metadata comment. WQP does not return comments.
658+
site_info : tuple[pd.DataFrame, WQP_Metadata] | None
659+
Site information if the query included a site filter (`siteid`,
660+
`sites`, `site`, or `site_no`).
660661
"""
661662

662663
def __init__(self, response, **parameters) -> None:
@@ -682,14 +683,14 @@ def __init__(self, response, **parameters) -> None:
682683

683684
self._parameters = parameters
684685

685-
@property
686-
def site_info(self):
687-
if "sites" in self._parameters:
688-
return what_sites(sites=parameters["sites"])
689-
elif "site" in self._parameters:
690-
return what_sites(sites=parameters["site"])
691-
elif "site_no" in self._parameters:
692-
return what_sites(sites=parameters["site_no"])
686+
@property
687+
def site_info(self):
688+
if "siteid" in self._parameters:
689+
return what_sites(siteid=self._parameters["siteid"])
690+
for legacy_key in ("sites", "site", "site_no"):
691+
if legacy_key in self._parameters:
692+
return what_sites(siteid=self._parameters[legacy_key])
693+
return None
693694

694695

695696
def _check_kwargs(kwargs):

tests/wqp_test.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,3 +216,42 @@ def test_check_kwargs():
216216
kwargs = {"mimeType": "foo"}
217217
with pytest.raises(ValueError):
218218
kwargs = _check_kwargs(kwargs)
219+
220+
221+
def test_wqp_metadata_site_info_property_resolves(requests_mock):
222+
"""`site_info` must be a real property bound to the class.
223+
224+
Regression: previously `site_info` was defined as a closure inside
225+
`__init__`, so `md.site_info` fell through to `BaseMetadata.site_info`
226+
and raised `NotImplementedError`. Also verify the parameters are
227+
threaded through from `get_results`.
228+
"""
229+
results_url = (
230+
"https://www.waterqualitydata.us/data/Result/Search?"
231+
"siteid=WIDNR_WQX-10032762&mimeType=csv"
232+
)
233+
sites_url = (
234+
"https://www.waterqualitydata.us/data/Station/Search?"
235+
"siteid=WIDNR_WQX-10032762&mimeType=csv"
236+
)
237+
mock_request(requests_mock, results_url, "tests/data/wqp_results.txt")
238+
mock_request(requests_mock, sites_url, "tests/data/wqp_sites.txt")
239+
240+
_df, md = get_results(siteid="WIDNR_WQX-10032762")
241+
242+
# site_info must be a bound property — accessing it should return the
243+
# what_sites() tuple, not raise NotImplementedError.
244+
site_df, site_md = md.site_info
245+
assert isinstance(site_df, DataFrame)
246+
assert site_md.url == sites_url
247+
248+
249+
def test_wqp_metadata_site_info_returns_none_without_site_filter(requests_mock):
250+
"""`site_info` returns None when no site filter was supplied."""
251+
results_url = (
252+
"https://www.waterqualitydata.us/data/Result/Search?"
253+
"characteristicName=Chloride&mimeType=csv"
254+
)
255+
mock_request(requests_mock, results_url, "tests/data/wqp_results.txt")
256+
_df, md = get_results(characteristicName="Chloride")
257+
assert md.site_info is None

0 commit comments

Comments
 (0)