Skip to content

Commit cecf4a4

Browse files
thodson-usgsthodson-usgs
committed
Implement final Copilot refinements: robust mocking, refined error handling, and defunct service logic
1 parent dd98537 commit cecf4a4

2 files changed

Lines changed: 39 additions & 31 deletions

File tree

‎dataretrieval/nwis.py‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -483,14 +483,15 @@ def get_dv(
483483
response = query_waterservices("dv", format="json", ssl_check=ssl_check, **kwargs)
484484
try:
485485
df = _read_json(response.json())
486-
except Exception as e:
486+
except (ValueError, requests.exceptions.JSONDecodeError) as e:
487487
if (
488488
"<html>" in response.text.lower()
489489
or "<!doctype" in response.text.lower()
490490
or "text/html" in response.headers.get("Content-Type", "").lower()
491491
):
492492
raise ValueError(
493-
"Received HTML response instead of JSON. This often indicates "
493+
f"Received HTML response instead of JSON from {response.url} "
494+
f"(Status: {response.status_code}). This often indicates "
494495
"that the service is currently unavailable."
495496
) from e
496497
raise
@@ -681,14 +682,15 @@ def get_iv(
681682

682683
try:
683684
df = _read_json(response.json())
684-
except Exception as e:
685+
except (ValueError, requests.exceptions.JSONDecodeError) as e:
685686
if (
686687
"<html>" in response.text.lower()
687688
or "<!doctype" in response.text.lower()
688689
or "text/html" in response.headers.get("Content-Type", "").lower()
689690
):
690691
raise ValueError(
691-
"Received HTML response instead of JSON. This often indicates "
692+
f"Received HTML response instead of JSON from {response.url} "
693+
f"(Status: {response.status_code}). This often indicates "
692694
"that the service is currently unavailable."
693695
) from e
694696
raise
@@ -913,6 +915,11 @@ def get_record(
913915
_check_sites_value_types(sites)
914916

915917
defunct_services = ["measurements", "gwlevels", "pmcodes", "water_use"]
918+
if service in defunct_services:
919+
raise NameError(
920+
f"The NWIS service '{service}' is no longer supported by get_record."
921+
)
922+
916923
if service not in WATERSERVICES_SERVICES + WATERDATA_SERVICES + defunct_services:
917924
raise TypeError(f"Unrecognized service: {service}")
918925

@@ -1240,15 +1247,13 @@ def site_info(self) -> tuple[pd.DataFrame, BaseMetadata] | None:
12401247
@property
12411248
def variable_info(self) -> None:
12421249
"""
1243-
Deprecated. Accessing variable_info via NWIS_Metadata is deprecated
1244-
as it relied on the defunct `get_pmcodes` function. Returns None.
1250+
Deprecated. Accessing variable_info via NWIS_Metadata is deprecated.
1251+
Returns None.
12451252
"""
1246-
# define variable_info metadata based on parameterCd if available
1247-
if "parameterCd" in self._parameters:
1248-
warnings.warn(
1249-
"Accessing variable_info via NWIS_Metadata is deprecated as "
1250-
"it relies on the defunct get_pmcodes function.",
1251-
DeprecationWarning,
1252-
stacklevel=2,
1253-
)
1254-
return None
1253+
warnings.warn(
1254+
"Accessing variable_info via NWIS_Metadata is deprecated as "
1255+
"it relies on the defunct get_pmcodes function.",
1256+
DeprecationWarning,
1257+
stacklevel=2,
1258+
)
1259+
return None

‎tests/nwis_test.py‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import datetime
22
import json
3+
from pathlib import Path
34
from unittest import mock
45

56
import numpy as np
@@ -29,7 +30,8 @@
2930

3031
def _load_mock_json(file_name):
3132
"""Helper to load mock JSON from tests/data."""
32-
with open(f"tests/data/{file_name}") as f:
33+
path = Path(__file__).parent / "data" / file_name
34+
with open(path, encoding="utf-8") as f:
3335
return json.load(f)
3436

3537

@@ -40,15 +42,15 @@ def _test_iv_service(requests_mock):
4042
service = "iv"
4143
site = ["03339000", "05447500", "03346500"]
4244

43-
# Minimal mock response
44-
mock_url = (
45-
"https://waterservices.usgs.gov/nwis/iv?format=json&"
46-
f"startDT={start}&endDT={end}&sites=03339000%2C05447500%2C03346500"
47-
)
4845
# We use a very simple JSON structure just to satisfy the parser
4946
mock_json = _load_mock_json("nwis_iv_mock.json")
5047

51-
requests_mock.get(mock_url, json=mock_json)
48+
# Match the base URL and ensure query parameters are correct
49+
requests_mock.get(
50+
"https://waterservices.usgs.gov/nwis/iv",
51+
json=mock_json,
52+
complete_qs=False,
53+
)
5254

5355
return get_record(site, start, end, service=service)
5456

@@ -139,19 +141,19 @@ def test_get_water_use_raises(self):
139141
get_water_use()
140142

141143
def test_get_record_defunct_service_measurements(self):
142-
with pytest.raises(NameError, match="get_discharge_measurements"):
144+
with pytest.raises(NameError, match="no longer supported by get_record"):
143145
get_record(service="measurements")
144146

145147
def test_get_record_defunct_service_gwlevels(self):
146-
with pytest.raises(NameError, match="get_gwlevels"):
148+
with pytest.raises(NameError, match="no longer supported by get_record"):
147149
get_record(service="gwlevels")
148150

149151
def test_get_record_defunct_service_pmcodes(self):
150-
with pytest.raises(NameError, match="get_pmcodes"):
152+
with pytest.raises(NameError, match="no longer supported by get_record"):
151153
get_record(service="pmcodes")
152154

153155
def test_get_record_defunct_service_water_use(self):
154-
with pytest.raises(NameError, match="get_water_use"):
156+
with pytest.raises(NameError, match="no longer supported by get_record"):
155157
get_record(service="water_use")
156158

157159

@@ -233,12 +235,13 @@ def test_empty_timeseries(requests_mock):
233235
start = "2010-07-20"
234236
end = "2010-07-20"
235237

236-
mock_url = (
237-
f"https://waterservices.usgs.gov/nwis/iv?format=json&"
238-
f"startDT={start}&endDT={end}&sites={sites}"
239-
)
240238
mock_json = _load_mock_json("nwis_iv_empty_mock.json")
241-
requests_mock.get(mock_url, json=mock_json)
239+
# Match the base URL and ensure query parameters are correct
240+
requests_mock.get(
241+
"https://waterservices.usgs.gov/nwis/iv",
242+
json=mock_json,
243+
complete_qs=False,
244+
)
242245

243246
df = get_record(sites=sites, service="iv", start=start, end=end)
244247
assert df.empty is True

0 commit comments

Comments
 (0)