Clean up review findings from ci-fix

thodson-usgs · claude · thodson-usgs · commit 8850b9956b97 · 2026-04-09T09:28:37.000-05:00
- Remove stray PR217.md tracking file
- Extract duplicated HTML-response detection into _parse_json_or_raise helper
- Remove dead defunct_services entry from get_record type check
- Point get_record defunct NameError to replacement functions
- Remove unreachable empty-dfs branch in _walk_pages

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/PR217.md b/PR217.md
diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py
@@ -45,6 +45,26 @@
 _CRS = "EPSG:4269"
 
 
+def _parse_json_or_raise(response: requests.Response) -> pd.DataFrame:
+    """Parse a JSON NWIS response, raising a helpful error on HTML responses."""
+    try:
+        return _read_json(response.json())
+    except (ValueError, JSONDecodeError) as e:
+        text_lower = response.text.lower()
+        content_type = response.headers.get("Content-Type", "").lower()
+        if (
+            "<html>" in text_lower
+            or "<!doctype" in text_lower
+            or "text/html" in content_type
+        ):
+            raise ValueError(
+                f"Received HTML response instead of JSON from {response.url} "
+                f"(Status: {response.status_code}). This often indicates "
+                "that the service is currently unavailable."
+            ) from e
+        raise
+
+
 def format_response(
     df: pd.DataFrame, service: str | None = None, **kwargs
 ) -> pd.DataFrame:
@@ -482,20 +502,7 @@ def get_dv(
     kwargs["multi_index"] = multi_index
 
     response = query_waterservices("dv", format="json", ssl_check=ssl_check, **kwargs)
-    try:
-        df = _read_json(response.json())
-    except (ValueError, JSONDecodeError) as e:
-        if (
-            "<html>" in response.text.lower()
-            or "<!doctype" in response.text.lower()
-            or "text/html" in response.headers.get("Content-Type", "").lower()
-        ):
-            raise ValueError(
-                f"Received HTML response instead of JSON from {response.url} "
-                f"(Status: {response.status_code}). This often indicates "
-                "that the service is currently unavailable."
-            ) from e
-        raise
+    df = _parse_json_or_raise(response)
 
     return format_response(df, **kwargs), NWIS_Metadata(response, **kwargs)
 
@@ -681,20 +688,7 @@ def get_iv(
         service="iv", format="json", ssl_check=ssl_check, **kwargs
     )
 
-    try:
-        df = _read_json(response.json())
-    except (ValueError, JSONDecodeError) as e:
-        if (
-            "<html>" in response.text.lower()
-            or "<!doctype" in response.text.lower()
-            or "text/html" in response.headers.get("Content-Type", "").lower()
-        ):
-            raise ValueError(
-                f"Received HTML response instead of JSON from {response.url} "
-                f"(Status: {response.status_code}). This often indicates "
-                "that the service is currently unavailable."
-            ) from e
-        raise
+    df = _parse_json_or_raise(response)
     return format_response(df, **kwargs), NWIS_Metadata(response, **kwargs)
 
 
@@ -915,13 +909,19 @@ def get_record(
     """
     _check_sites_value_types(sites)
 
-    defunct_services = ["measurements", "gwlevels", "pmcodes", "water_use"]
-    if service in defunct_services:
+    defunct_replacements = {
+        "measurements": "`waterdata.get_field_measurements`",
+        "gwlevels": "`waterdata.get_field_measurements`",
+        "pmcodes": "`waterdata.get_reference_table`",
+        "water_use": "no replacement available",
+    }
+    if service in defunct_replacements:
         raise NameError(
-            f"The NWIS service '{service}' is no longer supported by get_record."
+            f"The NWIS service '{service}' is no longer supported by "
+            f"get_record. Use {defunct_replacements[service]} instead."
         )
 
-    if service not in WATERSERVICES_SERVICES + WATERDATA_SERVICES + defunct_services:
+    if service not in WATERSERVICES_SERVICES + WATERDATA_SERVICES:
         raise TypeError(f"Unrecognized service: {service}")
 
     if service == "iv":
diff --git a/dataretrieval/waterdata/utils.py b/dataretrieval/waterdata/utils.py
@@ -610,9 +610,6 @@ def _walk_pages(
                 curr_url = None
 
         # Concatenate all pages at once for efficiency
-        if not dfs:
-            return pd.DataFrame(), initial_response
-
         return pd.concat(dfs, ignore_index=True), initial_response
     finally:
         if close_client:
