fix(nwis): forward get_record state, restore major-filter validation, clarify format

dataretrieval/nwis.py

@@ -130,6 +130,17 @@ def _parse_json_or_raise(response: httpx.Response) -> pd.DataFrame:
         raise
 
 
+def _reject_non_json_format(kwargs: dict, func_name: str) -> None:
+    """Drop ``format`` from ``kwargs`` and reject any non-JSON value.
+
+    These getters always request ``format="json"`` and parse the JSON body, so
+    a caller-supplied ``format`` (e.g. ``"rdb"``) would be silently overridden;
+    reject it explicitly instead.
+    """
+    if kwargs.pop("format", "json") != "json":
+        raise ValueError(f"{func_name} returns JSON and does not accept a `format`.")
+
+
 def format_response(
     df: pd.DataFrame, service: str | None = None, **kwargs
 ) -> pd.DataFrame:
@@ -276,7 +287,8 @@ def get_discharge_peaks(
     """
     _check_sites_value_types(sites)
 
-    kwargs["site_no"] = kwargs.pop("site_no", sites)
+    if sites is not None:
+        kwargs.setdefault("site_no", sites)
     kwargs["begin_date"] = kwargs.pop("begin_date", start)
     kwargs["end_date"] = kwargs.pop("end_date", end)
     kwargs["multi_index"] = multi_index
@@ -534,9 +546,11 @@ def get_dv(
 
     kwargs["startDT"] = kwargs.pop("startDT", start)
     kwargs["endDT"] = kwargs.pop("endDT", end)
-    kwargs["sites"] = kwargs.pop("sites", sites)
+    if sites is not None:
+        kwargs.setdefault("sites", sites)
     kwargs["multi_index"] = multi_index
 
+    _reject_non_json_format(kwargs, "get_dv")
     response = query_waterservices("dv", format="json", ssl_check=ssl_check, **kwargs)
     df = _parse_json_or_raise(response)
 
@@ -719,9 +733,11 @@ def get_iv(
 
     kwargs["startDT"] = kwargs.pop("startDT", start)
     kwargs["endDT"] = kwargs.pop("endDT", end)
-    kwargs["sites"] = kwargs.pop("sites", sites)
+    if sites is not None:
+        kwargs.setdefault("sites", sites)
     kwargs["multi_index"] = multi_index
 
+    _reject_non_json_format(kwargs, "get_iv")
     response = query_waterservices(
         service="iv", format="json", ssl_check=ssl_check, **kwargs
     )
@@ -952,6 +968,12 @@ def get_record(
     if service not in WATERSERVICES_SERVICES + WATERDATA_SERVICES:
         raise TypeError(f"Unrecognized service: {service}")
 
+    # Forward the documented `state` filter as the NWIS `stateCd` major filter;
+    # it was previously accepted but silently ignored, which produced a
+    # confusing "Bad Request" when used without `sites`.
+    if state is not None:
+        kwargs.setdefault("stateCd", state)
+
     if service == "iv":
         df, _ = get_iv(
             sites=sites,

tests/nwis_test.py

@@ -13,6 +13,7 @@
     NWIS_Metadata,
     _read_rdb,
     get_discharge_measurements,
+    get_dv,
     get_gwlevels,
     get_info,
     get_iv,
@@ -120,6 +121,46 @@ def test_preformat_peaks_response():
 # Removed defunct gwlevels tests.
 
 
+def test_get_dv_requires_major_filter():
+    """Regression: get_dv() with no major filter must raise the documented
+    TypeError. The getters injected ``kwargs["sites"] = kwargs.pop("sites",
+    sites)``, which always set the key (None when absent) and so defeated
+    query_waterservices' filter check, yielding a confusing Bad Request."""
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
+        with pytest.raises(TypeError, match="major filter"):
+            get_dv()
+
+
+def test_get_dv_rejects_non_json_format():
+    """Regression: get_dv passed format= explicitly alongside **kwargs, so
+    get_dv(..., format="rdb") raised 'multiple values for format'. It now
+    rejects a non-json format with a clear ValueError (it parses JSON)."""
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
+        with pytest.raises(ValueError, match="JSON"):
+            get_dv(sites="01646500", format="rdb")
+
+
+def test_get_record_forwards_state_as_statecd(monkeypatch):
+    """Regression: get_record's documented `state` arg was accepted but never
+    forwarded. It now reaches the request as the NWIS `stateCd` major filter."""
+    import dataretrieval.nwis as nwis_mod
+
+    captured: dict = {}
+
+    def fake_query_waterservices(service, format=None, ssl_check=True, **kw):
+        captured.update(kw)
+        raise RuntimeError("stop before network")
+
+    monkeypatch.setattr(nwis_mod, "query_waterservices", fake_query_waterservices)
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore")
+        with pytest.raises(RuntimeError, match="stop"):
+            get_record(state="OH", service="dv")
+    assert captured.get("stateCd") == "OH"
+
+
 class TestDeprecationWarnings:
     """Verify per-function DeprecationWarning fires with the right replacement.