Deprecate defunct NWIS functions, update tests, and improve 5xx error handling

thodson-usgs · thodson-usgs · commit 14a56f4a44a2 · 2026-04-03T13:08:21.000-05:00
diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py
@@ -481,11 +481,20 @@ def get_dv(
     kwargs["multi_index"] = multi_index
 
     response = query_waterservices("dv", format="json", ssl_check=ssl_check, **kwargs)
-    df = _read_json(response.json())
+    try:
+        df = _read_json(response.json())
+    except Exception as e:
+        if "<html>" in response.text.lower():
+            raise ValueError(
+                "Received HTML response instead of JSON. This often indicates "
+                "that the service is currently unavailable."
+            ) from e
+        raise e
 
     return format_response(df, **kwargs), NWIS_Metadata(response, **kwargs)
 
 
+
 def get_info(ssl_check: bool = True, **kwargs) -> tuple[pd.DataFrame, BaseMetadata]:
     """
     Get site description information from NWIS.
@@ -667,10 +676,19 @@ def get_iv(
         service="iv", format="json", ssl_check=ssl_check, **kwargs
     )
 
-    df = _read_json(response.json())
+    try:
+        df = _read_json(response.json())
+    except Exception as e:
+        if "<html>" in response.text.lower():
+            raise ValueError(
+                "Received HTML response instead of JSON. This often indicates "
+                "that the service is currently unavailable."
+            ) from e
+        raise e
     return format_response(df, **kwargs), NWIS_Metadata(response, **kwargs)
 
 
+
 def get_pmcodes(
     parameterCd: str | list[str] = "All",
     partial: bool = True,
@@ -840,11 +858,11 @@ def get_record(
         - 'iv' : instantaneous data
         - 'dv' : daily mean data
         - 'site' : site description
-        - 'measurements' : discharge measurements
+        - 'measurements' : (defunct) use `waterdata.get_field_measurements`
         - 'peaks': discharge peaks
-        - 'gwlevels': groundwater levels
-        - 'pmcodes': get parameter codes
-        - 'water_use': get water use data
+        - 'gwlevels': (defunct) use `waterdata.get_field_measurements`
+        - 'pmcodes': (defunct) use `get_reference_table`
+        - 'water_use': (defunct) defunct
         - 'ratings': get rating table
         - 'stat': get statistics
     ssl_check: bool, optional
@@ -870,29 +888,12 @@ def get_record(
         >>> # Get site description for site 01585200
         >>> df = dataretrieval.nwis.get_record(sites="01585200", service="site")
 
-        >>> # Get discharge measurements for site 01585200
-        >>> df = dataretrieval.nwis.get_record(
-        ...     sites="01585200", service="measurements"
-        ... )
+        >>> # Get site description for site 01585200
+        >>> df = dataretrieval.nwis.get_record(sites="01585200", service="site")
 
         >>> # Get discharge peaks for site 01585200
         >>> df = dataretrieval.nwis.get_record(sites="01585200", service="peaks")
 
-        >>> # Get latest groundwater level for site 434400121275801
-        >>> df = dataretrieval.nwis.get_record(
-        ...     sites="434400121275801", service="gwlevels"
-        ... )
-
-        >>> # Get information about the discharge parameter code
-        >>> df = dataretrieval.nwis.get_record(
-        ...     service="pmcodes", parameterCd="00060"
-        ... )
-
-        >>> # Get water use data for livestock nationally in 2010
-        >>> df = dataretrieval.nwis.get_record(
-        ...     service="water_use", years="2010", categories="L"
-        ... )
-
         >>> # Get rating table for USGS streamgage 01585200
         >>> df = dataretrieval.nwis.get_record(sites="01585200", service="ratings")
 
@@ -907,7 +908,8 @@ def get_record(
     """
     _check_sites_value_types(sites)
 
-    if service not in WATERSERVICES_SERVICES + WATERDATA_SERVICES:
+    defunct_services = ["measurements", "gwlevels", "pmcodes", "water_use"]
+    if service not in WATERSERVICES_SERVICES + WATERDATA_SERVICES + defunct_services:
         raise TypeError(f"Unrecognized service: {service}")
 
     if service == "iv":
@@ -1235,4 +1237,10 @@ def site_info(self) -> tuple[pd.DataFrame, BaseMetadata] | None:
     def variable_info(self) -> tuple[pd.DataFrame, BaseMetadata] | None:
         # define variable_info metadata based on parameterCd if available
         if "parameterCd" in self._parameters:
-            return get_pmcodes(parameterCd=self._parameters["parameterCd"])
+            warnings.warn(
+                "Accessing variable_info via NWIS_Metadata is deprecated as "
+                "it relies on the defunct get_pmcodes function.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
+            return None
diff --git a/dataretrieval/utils.py b/dataretrieval/utils.py
@@ -212,6 +212,12 @@ def query(url, payload, delimiter=",", ssl_check=True):
             + f"API response reason: {_reason}. Pseudo-code example of how to "
             + f"split your query: \n {_example}"
         )
+    elif response.status_code in [500, 502, 503]:
+        raise ValueError(
+            f"Service Unavailable: {response.status_code} {response.reason}. "
+            + f"The service at {response.url} may be down or experiencing issues."
+        )
+
 
     if response.text.startswith("No sites/data"):
         raise NoSitesError(response.url)
diff --git a/tests/nwis_test.py b/tests/nwis_test.py
@@ -7,9 +7,14 @@
 
 from dataretrieval.nwis import (
     NWIS_Metadata,
+    get_discharge_measurements,
+    get_gwlevels,
     get_info,
     get_iv,
+    get_pmcodes,
+    get_qwdata,
     get_record,
+    get_water_use,
     preformat_peaks_response,
     what_sites,
 )
@@ -21,24 +26,69 @@
 SITENO_COL = "site_no"
 
 
-def test_iv_service():
-    """Unit test of instantaneous value service"""
+def _test_iv_service(requests_mock):
+    """Mocked test of instantaneous value service"""
     start = START_DATE
     end = END_DATE
     service = "iv"
     site = ["03339000", "05447500", "03346500"]
+    
+    # Minimal mock response
+    mock_url = (
+        "https://waterservices.usgs.gov/nwis/iv?format=json&"
+        f"startDT={start}&endDT={end}&sites=03339000%2C05447500%2C03346500"
+    )
+    # We use a very simple JSON structure just to satisfy the parser
+    mock_json = {
+        "value": {
+            "timeSeries": [
+                {
+                    "sourceInfo": {"siteCode": [{"value": "03339000"}]},
+                    "variable": {"variableCode": [{"value": "00060"}], "options": {"option": [{"value": "mean"}]}},
+                    "values": [{"method": [{"methodDescription": "mean"}], "value": [{"value": "1.0", "dateTime": "2018-01-24T00:00:00Z", "qualifiers": "A"}]}]
+                },
+                {
+                    "sourceInfo": {"siteCode": [{"value": "05447500"}]},
+                    "variable": {"variableCode": [{"value": "00060"}], "options": {"option": [{"value": "mean"}]}},
+                    "values": [{"method": [{"methodDescription": "mean"}], "value": [{"value": "2.0", "dateTime": "2018-01-24T00:00:00Z", "qualifiers": "A"}]}]
+                }
+            ]
+        }
+    }
+
+    requests_mock.get(mock_url, json=mock_json)
+    
     return get_record(site, start, end, service=service)
 
 
-def test_iv_service_answer():
-    df = test_iv_service()
+def test_iv_service_answer(requests_mock):
+    df = _test_iv_service(requests_mock)
     # check multiindex function
     assert df.index.names == [
         SITENO_COL,
         DATETIME_COL,
     ], f"iv service returned incorrect index: {df.index.names}"
 
 
+def test_nwis_service_live():
+    """Live sanity check of NWIS service, tolerant of 502/503."""
+    site = "01491000"
+    try:
+        # Minimal query: just most recent record
+        get_iv(sites=site)
+    except ValueError as e:
+        # Catch our custom 5xx error from utils.py
+        if any(err in str(e) for err in ["502", "503", "Service Unavailable"]):
+            pytest.skip(f"Service is currently unavailable (transient 502/503): {e}")
+        raise e
+    except Exception as e:
+        # Fallback for other potential transient network issues
+        if "Expecting value" in str(e) or "JSON" in str(e):
+            pytest.skip(f"Service returned invalid response (likely 502/503): {e}")
+        raise e
+
+
+
 def test_preformat_peaks_response():
     # make a data frame with a "peak_dt" datetime column
     # it will have some nan and none values
@@ -61,82 +111,48 @@ def test_preformat_peaks_response():
 # incomplete date-time information
 
 
-@pytest.mark.xfail(reason="Modern service does not return incomplete dates.")
-def test_inc_date_01():
-    """Test based on GitHub Issue #47 - lack of timestamp for measurement."""
-    site = "403451073585601"
-    # make call expecting a warning to be thrown due to incomplete dates
-    with pytest.warns(UserWarning) as record:
-        df = get_record(site, "1980-01-01", "1990-01-01", service="gwlevels")
-
-    if len(df) == 0:
-        pytest.skip(f"Site {site} returned no data.")
-
-    assert len(record) > 0
-    # assert that there are indeed incomplete dates
-    assert pd.isna(df.index).any()
-    # assert that the datetime index is there
-    assert df.index.name == "datetime"
-    # make call without defining a datetime index and check that it isn't there
-    df2 = get_record(
-        site, "1980-01-01", "1990-01-01", service="gwlevels", datetime_index=False
-    )
-    # assert shape of both dataframes is the same (contain the same data)
-    assert df.shape == df2.shape
-    # assert that the datetime index is not there
-    assert df2.index.name != "datetime"
-
-
-@pytest.mark.xfail(reason="Modern service does not return incomplete dates.")
-def test_inc_date_02():
-    """Test based on GitHub Issue #47 - lack of month, day, or time."""
-    site = "180049066381200"
-    # make call expecting a warning to be thrown due to incomplete dates
-    with pytest.warns(UserWarning) as record:
-        df = get_record(site, "1900-01-01", "2013-01-01", service="gwlevels")
-
-    if len(df) == 0:
-        pytest.skip(f"Site {site} returned no data.")
-
-    assert len(record) > 0
-    # assert that there are indeed incomplete dates
-    assert pd.isna(df.index).any()
-    # assert that the datetime index is there
-    assert df.index.name == "datetime"
-    # make call without defining a datetime index and check that it isn't there
-    df2 = get_record(
-        site, "1900-01-01", "2013-01-01", service="gwlevels", datetime_index=False
-    )
-    # assert shape of both dataframes is the same (contain the same data)
-    assert df.shape == df2.shape
-    # assert that the datetime index is not there
-    assert df2.index.name != "datetime"
-
-
-@pytest.mark.xfail(reason="Modern service does not return incomplete dates.")
-def test_inc_date_03():
-    """Test based on GitHub Issue #47 - lack of day, and times."""
-    site = "290000095192602"
-    # make call expecting a warning to be thrown due to incomplete dates
-    with pytest.warns(UserWarning) as record:
-        df = get_record(site, "1975-01-01", "2000-01-01", service="gwlevels")
-
-    if len(df) == 0:
-        pytest.skip(f"Site {site} returned no data.")
-
-    assert len(record) > 0
-    # assert that there are indeed incomplete dates
-    assert pd.isna(df.index).any()
-    # assert that the datetime index is there
-    assert df.index.name == "datetime"
-    # make call without defining a datetime index and check that it isn't there
-    df2 = get_record(
-        site, "1975-01-01", "2000-01-01", service="gwlevels", datetime_index=False
-    )
-    # assert shape of both dataframes is the same (contain the same data)
-    assert df.shape == df2.shape
-    # assert that the datetime index is not there
-    assert df2.index.name != "datetime"
+# Removed defunct gwlevels tests.
+
+
+class TestDefunct:
+    """Verify that defunct functions raise NameError."""
+
+    def test_get_qwdata_raises(self):
+        with pytest.raises(NameError, match="get_qwdata"):
+            get_qwdata()
+
+    def test_get_discharge_measurements_raises(self):
+        with pytest.raises(NameError, match="get_discharge_measurements"):
+            get_discharge_measurements()
+
+    def test_get_gwlevels_raises(self):
+        with pytest.raises(NameError, match="get_gwlevels"):
+            get_gwlevels()
+
+    def test_get_pmcodes_raises(self):
+        with pytest.raises(NameError, match="get_pmcodes"):
+            get_pmcodes()
+
+    def test_get_water_use_raises(self):
+        with pytest.raises(NameError, match="get_water_use"):
+            get_water_use()
+
+    def test_get_record_defunct_service_measurements(self):
+        with pytest.raises(NameError, match="get_discharge_measurements"):
+            get_record(service="measurements")
+
+    def test_get_record_defunct_service_gwlevels(self):
+        with pytest.raises(NameError, match="get_gwlevels"):
+            get_record(service="gwlevels")
+
+    def test_get_record_defunct_service_pmcodes(self):
+        with pytest.raises(NameError, match="get_pmcodes"):
+            get_record(service="pmcodes")
+
+    def test_get_record_defunct_service_water_use(self):
+        with pytest.raises(NameError, match="get_water_use"):
+            get_record(service="water_use")
+
 
 
 class TestTZ:
@@ -211,14 +227,26 @@ def test_expandedrdb_get_info(self):
         assert "count_nu" not in data.columns
 
 
-def test_empty_timeseries():
+def test_empty_timeseries(requests_mock):
     """Test based on empty case from GitHub Issue #26."""
+    sites = "011277906"
+    start = "2010-07-20"
+    end = "2010-07-20"
+    
+    mock_url = (
+        f"https://waterservices.usgs.gov/nwis/iv?format=json&"
+        f"startDT={start}&endDT={end}&sites={sites}"
+    )
+    mock_json = {"value": {"timeSeries": []}}
+    requests_mock.get(mock_url, json=mock_json)
+    
     df = get_record(
-        sites="011277906", service="iv", start="2010-07-20", end="2010-07-20"
+        sites=sites, service="iv", start=start, end=end
     )
     assert df.empty is True
 
 
+
 class TestMetaData:
     """Tests of NWIS metadata setting,
 
