From 1dd136f4de4e49d349ba409c8c9d635504cbf036 Mon Sep 17 00:00:00 2001 From: Arpit Jain Date: Fri, 17 Jul 2026 06:03:15 +0900 Subject: [PATCH 1/2] fix(nwis): handle empty peaks response instead of raising KeyError nwis.get_discharge_peaks / get_record(service="peaks") against a site with no annual-peak data returns a peaks RDB body of comment lines only, which read_rdb parses to a column-less empty DataFrame. format_response runs preformat_peaks_response before its own empty-frame check, and that function's first statement pops "peak_dt", so the empty case raised KeyError('peak_dt') instead of returning an empty frame. This is the same empty-result contract fixed for the other services in issue #171; peaks was missed because it is preformatted first. Return the frame unchanged when peak_dt is absent so the empty-frame path in format_response handles it and callers can check df.empty. Adds a regression test alongside the existing #171 coverage. Signed-off-by: Arpit Jain --- dataretrieval/nwis.py | 7 +++++++ tests/nwis_test.py | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py index 25e9cf8f..255ebbe8 100644 --- a/dataretrieval/nwis.py +++ b/dataretrieval/nwis.py @@ -196,6 +196,13 @@ def preformat_peaks_response(df: pd.DataFrame) -> pd.DataFrame: The formatted data frame """ + if "peak_dt" not in df.columns: + # An empty peaks response (e.g. "No sites found") parses to a + # column-less frame, so there is no peak_dt to reformat. Return it + # unchanged and let format_response's empty-frame path handle it, + # matching how the other services treat empty results (issue #171). + return df + df["datetime"] = pd.to_datetime(df.pop("peak_dt"), errors="coerce") df.dropna(subset=["datetime"], inplace=True) return df diff --git a/tests/nwis_test.py b/tests/nwis_test.py index 905ed8db..f95b4076 100644 --- a/tests/nwis_test.py +++ b/tests/nwis_test.py @@ -11,6 +11,7 @@ from dataretrieval.nwis import ( NWIS_Metadata, _read_rdb, + format_response, get_discharge_measurements, get_gwlevels, get_iv, @@ -303,3 +304,20 @@ def test_no_sites_flows_through_format_response(self): df = _read_rdb(no_sites_rdb) assert isinstance(df, pd.DataFrame) assert df.empty + + def test_no_peaks_flows_through_format_response(self): + """The 'peaks' service takes an extra formatting step + (preformat_peaks_response) before the empty-frame check, so an empty + peaks response has to survive that too. Previously this raised + KeyError('peak_dt'); now it returns an empty frame like the other + services (same empty-result contract as issue #171). + """ + no_peaks_rdb = ( + "# //Output-Format: RDB\n" + "# //Response-Status: OK\n" + "# //Response-Message: No sites found matching all criteria\n" + ) + df = _read_rdb(no_peaks_rdb) + df = format_response(df, service="peaks") + assert isinstance(df, pd.DataFrame) + assert df.empty From 751d3b326d99fb60a76d96d7774e25f58a05eb6c Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Thu, 16 Jul 2026 16:22:32 -0500 Subject: [PATCH 2/2] test(nwis): parse with raw read_rdb in empty-peaks regression test The empty-peaks test parsed with _read_rdb, which already runs format_response(service=None); the real get_discharge_peaks path uses the raw read_rdb parser followed by format_response(service="peaks"). Switch to read_rdb so the test exercises the actual call path without the redundant format pass. Signed-off-by: thodson-usgs Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/nwis_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/nwis_test.py b/tests/nwis_test.py index f95b4076..9bb24bf7 100644 --- a/tests/nwis_test.py +++ b/tests/nwis_test.py @@ -20,6 +20,7 @@ get_record, get_water_use, preformat_peaks_response, + read_rdb, ) START_DATE = "2018-01-24" @@ -317,7 +318,9 @@ def test_no_peaks_flows_through_format_response(self): "# //Response-Status: OK\n" "# //Response-Message: No sites found matching all criteria\n" ) - df = _read_rdb(no_peaks_rdb) + # Mirror get_discharge_peaks: raw read_rdb, then the peaks-specific + # format_response (not _read_rdb, which formats with service=None). + df = read_rdb(no_peaks_rdb) df = format_response(df, service="peaks") assert isinstance(df, pd.DataFrame) assert df.empty