Address Copilot review on PR #229

1. TypeError messages now use a fixed example ("USGS-01646500") instead
   of interpolating the user's value into the suggestion template.
   The offending value is still surfaced via "(got {value!r})" so the
   user can see what they actually passed — but mappings, large objects,
   etc. no longer produce ugly suggestion strings like "USGS-{'k': 'v'}".

2. _MONITORING_LOCATION_ID_RE switched from anchored `^.+-.+$` matched
   with `re.match` to un-anchored `.+-.+` matched with `re.fullmatch`.
   Same effective behavior, but `fullmatch` makes the intent explicit
   and removes the redundant anchor characters.

3. Widened the type annotations on all 10 public waterdata functions
   that accept `monitoring_location_id` from `str | list[str] | None`
   to `str | Iterable[str] | None`, matching the runtime contract that
   accepts tuples, pandas.Series, pandas.Index, numpy.ndarray, etc.
   Added `from collections.abc import Iterable` to api.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

dataretrieval/waterdata/api.py

@@ -8,6 +8,7 @@
 
 import json
 import logging
+from collections.abc import Iterable
 from io import StringIO
 from typing import get_args
 
@@ -38,7 +39,7 @@
 
 
 def get_daily(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     parameter_code: str | list[str] | None = None,
     statistic_id: str | list[str] | None = None,
     properties: list[str] | None = None,
@@ -225,7 +226,7 @@ def get_daily(
 
 
 def get_continuous(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     parameter_code: str | list[str] | None = None,
     statistic_id: str | list[str] | None = None,
     properties: list[str] | None = None,
@@ -414,7 +415,7 @@ def get_continuous(
 
 
 def get_monitoring_locations(
-    monitoring_location_id: list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     agency_code: list[str] | None = None,
     agency_name: list[str] | None = None,
     monitoring_location_number: list[str] | None = None,
@@ -713,7 +714,7 @@ def get_monitoring_locations(
 
 
 def get_time_series_metadata(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     parameter_code: str | list[str] | None = None,
     parameter_name: str | list[str] | None = None,
     properties: str | list[str] | None = None,
@@ -937,7 +938,7 @@ def get_time_series_metadata(
 
 
 def get_latest_continuous(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     parameter_code: str | list[str] | None = None,
     statistic_id: str | list[str] | None = None,
     properties: str | list[str] | None = None,
@@ -1117,7 +1118,7 @@ def get_latest_continuous(
 
 
 def get_latest_daily(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     parameter_code: str | list[str] | None = None,
     statistic_id: str | list[str] | None = None,
     properties: str | list[str] | None = None,
@@ -1299,7 +1300,7 @@ def get_latest_daily(
 
 
 def get_field_measurements(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     parameter_code: str | list[str] | None = None,
     observing_procedure_code: str | list[str] | None = None,
     properties: list[str] | None = None,
@@ -1816,7 +1817,7 @@ def get_stats_por(
     county_code: str | list[str] | None = None,
     start_date: str | None = None,
     end_date: str | None = None,
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     page_size: int = 1000,
     parent_time_series_id: str | list[str] | None = None,
     site_type_code: str | list[str] | None = None,
@@ -1941,7 +1942,7 @@ def get_stats_date_range(
     county_code: str | list[str] | None = None,
     start_date: str | None = None,
     end_date: str | None = None,
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     page_size: int = 1000,
     parent_time_series_id: str | list[str] | None = None,
     site_type_code: str | list[str] | None = None,
@@ -2066,7 +2067,7 @@ def get_stats_date_range(
 
 
 def get_channel(
-    monitoring_location_id: str | list[str] | None = None,
+    monitoring_location_id: str | Iterable[str] | None = None,
     field_visit_id: str | list[str] | None = None,
     measurement_number: str | list[str] | None = None,
     time: str | list[str] | None = None,

dataretrieval/waterdata/utils.py

@@ -1153,7 +1153,7 @@ def _check_profiles(
         )
 
 
-_MONITORING_LOCATION_ID_RE = re.compile(r"^.+-.+$")
+_MONITORING_LOCATION_ID_RE = re.compile(r".+-.+")
 
 
 def _check_monitoring_location_id(
@@ -1198,25 +1198,26 @@ def _check_monitoring_location_id(
     ):
         raise TypeError(
             f"monitoring_location_id must be a string or iterable of strings, "
-            f"not {type(monitoring_location_id).__name__}. "
-            f"Expected format: 'AGENCY-ID', e.g., 'USGS-{monitoring_location_id}'."
+            f"not {type(monitoring_location_id).__name__} "
+            f"(got {monitoring_location_id!r}). "
+            f"Expected 'AGENCY-ID' format, e.g., 'USGS-01646500'."
         )
 
     ids = list(monitoring_location_id)
     for id_ in ids:
         if not isinstance(id_, str):
             raise TypeError(
                 f"monitoring_location_id elements must be strings, "
-                f"not {type(id_).__name__}. "
-                f"Expected format: 'AGENCY-ID', e.g., 'USGS-{id_}'."
+                f"not {type(id_).__name__} (got {id_!r}). "
+                f"Expected 'AGENCY-ID' format, e.g., 'USGS-01646500'."
             )
         _check_id_format(id_)
     return ids
 
 
 def _check_id_format(value: str) -> None:
     """Raise ``ValueError`` if ``value`` is not in ``AGENCY-ID`` format."""
-    if not _MONITORING_LOCATION_ID_RE.match(value):
+    if not _MONITORING_LOCATION_ID_RE.fullmatch(value):
         raise ValueError(
             f"Invalid monitoring_location_id: {value!r}. "
             f"Expected 'AGENCY-ID' format, e.g., 'USGS-01646500'."