fix(nwis): keep censored peaks instead of dropping or dating them

NEWS.md

@@ -1,3 +1,7 @@
+**08/27/2026:** **Bug fix:** `nwis.get_discharge_peaks` and `nwis.get_record(service='peaks')` silently discarded every peak whose date is only partly known. NWIS zero-fills the unknown part of a historical peak's date -- `YYYY-MM-00` when the day is not known, `YYYY-00-00` when the month is not either (the `Bd` and `Bm` `peak_cd` qualifiers) -- and neither parses as a date, so `preformat_peaks_response` coerced both to `NaT` and then dropped the row along with its discharge value. These are real peaks, and disproportionately a site's largest: site 14105700 lost 20 of its 167 peaks, among them an 1859 flood of 847,000 ft3/s, and 06934500 lost its 1844 peak of 700,000 ft3/s. Such a peak is now kept, with `datetime` left as `NaT`. The date is not completed into one NWIS does not have: a `datetime64` column cannot hold a partial date, so any value there would assert a day the record does not claim. **Behavior change:** peaks queries return more rows than before, and `datetime` may now be `NaT` -- a caller selecting on the datetime index will not see those peaks and should filter on `peak_dt` instead. **Behavior change:** `peak_dt` is no longer removed from the returned frame. It is the only column carrying a censored peak's year, since the peaks response has no `water_yr`, and the only dependable way to tell an unknown day from a known one -- `peak_cd` does not always carry the qualifier (22 of 24 censored dates across six sites tested). For peaks with a resolved timestamp alongside explicit `year`/`month`/`day` and a `qualifier` field, use `waterdata.get_peaks()`.
+
+**08/26/2026:** **Bug fix:** `nwis.format_response(df, service='peaks')` and `nwis.preformat_peaks_response` raised `KeyError('peak_dt')` on an empty peaks response instead of returning an empty frame. Both are public, and every other service already treated an empty result as a legitimate empty frame rather than an error (issue #171); the peaks arm slipped through because it reformats the datetime column before the empty-frame check. Callers can now check `df.empty` rather than catching an exception. A *non-empty* frame with no `peak_dt` column is malformed rather than empty, and still raises.
+
 **08/25/2026:** Removed `dataretrieval.ogc.retry`, which only re-exported private helpers. Deprecated `dataretrieval.ogc.interruptions`; import exceptions from `dataretrieval` or `dataretrieval.interruptions` instead. The old path will be removed in a future major release, no earlier than 2027-08-25.
 
 **08/20/2026:** The `state` filter now accepts the five US territories. `dataretrieval.codes.states` held the 50 states and DC, so `ngwmn.get_sites(state='Puerto Rico')`, `waterdata.get_monitoring_locations(state_name='Puerto Rico')` via the unified `state`, and `nwdc.get_wateruse(state='PR')` were refused locally -- while all three services carry the data (NGWMN answers with 36 Puerto Rico monitoring locations, the Water Data monitoring-locations collection returns Puerto Rico sites, and legacy NWIS lists 1,148 stream sites for `stateCd=PR`). American Samoa, Guam, the Northern Mariana Islands, Puerto Rico and the US Virgin Islands are now in both code tables under their real ANSI/FIPS codes, so every encoding resolves: `'Puerto Rico'`, `'PR'`, `'72'` and `'US:72'` all normalize alike. **Behavior change:** a territory that used to raise `ValueError` now produces a request. A value the table genuinely does not hold still fails fast.

dataretrieval/nwis.py

@@ -217,9 +217,26 @@ def preformat_peaks_response(df: pd.DataFrame) -> pd.DataFrame:
     df: ``pandas.DataFrame``
         The formatted data frame.
 
+    Notes
+    -----
+    An empty frame with no ``peak_dt`` column is returned unchanged, so an
+    empty peaks response reaches :func:`format_response`'s empty-frame path
+    instead of raising ``KeyError``.
+
+    NWIS zero-fills the unknown part of a historical peak's date --
+    ``YYYY-MM-00`` when the day is not known, ``YYYY-00-00`` when the month is
+    not either. Neither parses, so ``datetime`` is ``NaT`` for those peaks
+    rather than a date NWIS does not have. The peak is kept regardless, and
+    ``peak_dt`` is left in the frame: the response carries no ``water_yr``, so
+    it is the only column holding a censored peak's year.
+
     """
-    df["datetime"] = pd.to_datetime(df.pop("peak_dt"), errors="coerce")
-    df.dropna(subset=["datetime"], inplace=True)
+    if df.empty and "peak_dt" not in df.columns:
+        # A non-empty frame missing peak_dt is malformed, not empty, and must
+        # still raise.
+        return df
+
+    df["datetime"] = pd.to_datetime(df["peak_dt"], errors="coerce")
     return df
 
 

tests/nwis_test.py

@@ -13,6 +13,7 @@
 from dataretrieval.nwis import (
     NWIS_Metadata,
     _read_rdb,
+    format_response,
     get_discharge_measurements,
     get_gwlevels,
     get_iv,
@@ -22,6 +23,7 @@
     get_water_use,
     preformat_peaks_response,
 )
+from dataretrieval.rdb import read_rdb
 
 START_DATE = "2018-01-24"
 END_DATE = "2018-01-25"
@@ -79,17 +81,63 @@ def test_iv_service_answer(httpx_mock):
     ], f"iv service returned incorrect index: {df.index.names}"
 
 
-def test_preformat_peaks_response():
-    # make a data frame with a "peak_dt" datetime column
-    # it will have some nan and none values
-    data = {"peak_dt": ["2000-03-22", np.nan, None], "peak_va": [1000, 2000, 3000]}
-    # turn data into dataframe
-    df = pd.DataFrame(data)
-    # run preformat function
+@pytest.mark.parametrize(
+    "peak_dt, expected",
+    [
+        ("1878-06-12", "1878-06-12"),  # fully known
+        ("1844-06-00", None),  # day unknown (peak_cd Bd)
+        ("1858-00-00", None),  # month unknown (peak_cd Bm)
+        ("", None),  # no date at all
+        (np.nan, None),
+    ],
+)
+def test_preformat_peaks_response_keeps_every_peak(peak_dt, expected):
+    """A peak is never dropped for want of a parseable date.
+
+    NWIS zero-fills the unknown part of a historical peak's date --
+    ``YYYY-MM-00`` when the day is not known, ``YYYY-00-00`` when the month is
+    not either (the ``Bd`` and ``Bm`` ``peak_cd`` qualifiers). Those are real
+    peaks, often a site's largest, and dropping them loses the discharge value
+    with the date. A date NWIS only partly knows stays ``NaT`` rather than
+    being completed into one it does not have.
+    """
+    df = pd.DataFrame({"peak_dt": [peak_dt], "peak_va": [563000]})
+
+    df = preformat_peaks_response(df)
+
+    assert len(df) == 1, f"{peak_dt!r} was dropped"
+    assert df["peak_va"].iloc[0] == 563000
+    if expected is None:
+        assert pd.isna(df["datetime"].iloc[0])
+    else:
+        assert df["datetime"].iloc[0] == pd.Timestamp(expected)
+
+
+def test_preformat_peaks_response_preserves_peak_dt():
+    """``peak_dt`` must survive the reformat.
+
+    The peaks response carries no ``water_yr``, so ``peak_dt`` is the only
+    column holding the year of a censored peak -- and the only way a caller can
+    tell an unknown day from a known one, since ``peak_cd`` does not always
+    carry the qualifier.
+    """
+    df = pd.DataFrame({"peak_dt": ["1858-00-00"], "peak_va": [563000]})
+
     df = preformat_peaks_response(df)
-    # assertions
-    assert "datetime" in df.columns
-    assert df["datetime"].isna().sum() == 0
+
+    assert df["peak_dt"].iloc[0] == "1858-00-00"
+
+
+def test_preformat_peaks_response_malformed_frame_still_raises():
+    """Only an *empty* peaks frame is a legitimate empty result. A non-empty
+    frame with no ``peak_dt`` column is a malformed response -- a truncated or
+    altered RDB header -- and must stay loud rather than be returned silently
+    without its datetime index.
+    """
+    df = pd.DataFrame({"peak_va": [1000]})
+
+    with pytest.raises(KeyError, match="peak_dt"):
+        format_response(df, service="peaks")
 
 
 class TestDeprecationWarnings:
@@ -288,21 +336,41 @@ class TestReadRdb:
 
     The format-agnostic parser is exercised in tests/rdb_test.py; this
     class pins the wrapper-specific contract — that an empty parser
-    result flows through format_response without crashing (issue #171).
+    result flows through format_response without crashing (issue #171),
+    on the plain arm and on the peaks arm alike.
     """
 
+    NO_RESULTS_RDB = (
+        "# //Output-Format: RDB\n"
+        "# //Response-Status: OK\n"
+        "# //Response-Message: No sites found matching all criteria\n"
+    )
+
     def test_no_sites_flows_through_format_response(self):
         """A "No sites found" response is a legitimate empty result, not an
         error, so callers can check ``df.empty`` rather than catching an
         exception. Regression for issue #171 (previously raised IndexError),
         which now also covers the empty-frame path through ``format_response``.
         """
-        no_sites_rdb = (
-            "# //Output-Format: RDB\n"
-            "# //Response-Status: OK\n"
-            "# //Response-Message: No sites found matching all criteria\n"
-        )
-        df = _read_rdb(no_sites_rdb)
+        df = _read_rdb(self.NO_RESULTS_RDB)
+        assert isinstance(df, pd.DataFrame)
+        assert df.empty
+
+    def test_no_peaks_flows_through_format_response(self):
+        """``format_response(service="peaks")`` must tolerate an empty frame.
+
+        The peaks arm runs ``preformat_peaks_response`` before the
+        "datetime not in columns" check, and that function popped ``peak_dt``
+        unconditionally, so a column-less frame raised ``KeyError`` where every
+        other service returned an empty frame (issue #171's contract).
+
+        Both functions are public API, so any caller parsing a peaks RDB
+        reaches this -- it is not unreachable behind ``NoSitesError``.
+        """
+        # Mirror get_discharge_peaks: raw read_rdb, then the peaks-specific
+        # format_response.
+        df = read_rdb(self.NO_RESULTS_RDB)
+        df = format_response(df, service="peaks")
         assert isinstance(df, pd.DataFrame)
         assert df.empty
 

tests/waterservices_test.py

@@ -242,7 +242,7 @@ def test_get_discharge_peaks(httpx_mock):
     if not isinstance(df, DataFrame):
         raise TypeError(f"{type(df)} is not DataFrame base class type")
 
-    assert df.size == 240
+    assert df.shape == (20, 13)
     assert_metadata(httpx_mock, request_url, md, site, None, format)
 
 
@@ -267,7 +267,7 @@ def test_get_discharge_peaks_sites_value_types(httpx_mock, site_input_type_list)
     if not isinstance(df, DataFrame):
         raise TypeError(f"{type(df)} is not DataFrame base class type")
 
-    assert df.size == 240
+    assert df.shape == (20, 13)
 
 
 def test_get_ratings_validation():