diff --git a/NEWS.md b/NEWS.md index 6dfb3ba9..f025a999 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**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. diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py index 44d74828..86b32824 100644 --- a/dataretrieval/nwis.py +++ b/dataretrieval/nwis.py @@ -217,7 +217,20 @@ 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 that + an empty peaks response reaches :func:`format_response`'s empty-frame path + rather than raising ``KeyError``. + """ + if df.empty and "peak_dt" not in df.columns: + # An empty response parses to a column-less frame; return it so + # format_response's empty-frame path handles it like every other + # service. A non-empty frame missing peak_dt is malformed, not empty, + # and still raises. + 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 9c424fa0..f479e951 100644 --- a/tests/nwis_test.py +++ b/tests/nwis_test.py @@ -11,8 +11,10 @@ from dataretrieval import nwis from dataretrieval.exceptions import DataCurrencyWarning from dataretrieval.nwis import ( + _NWIS_RDB_DTYPES, NWIS_Metadata, _read_rdb, + format_response, get_discharge_measurements, get_gwlevels, get_iv, @@ -22,6 +24,7 @@ get_water_use, preformat_peaks_response, ) +from dataretrieval.rdb import read_rdb START_DATE = "2018-01-24" END_DATE = "2018-01-25" @@ -284,11 +287,12 @@ def test_set_metadata_info_countyCd(self, httpx_mock): class TestReadRdb: - """Tests for the NWIS-specific _read_rdb wrapper. + """Tests for the NWIS-specific parse-then-format path. 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). + class pins the NWIS-specific contract — that an empty parser result + flows through format_response without crashing (issue #171), on the + plain arm via _read_rdb and on the peaks arm via format_response. """ def test_no_sites_flows_through_format_response(self): @@ -306,6 +310,43 @@ def test_no_sites_flows_through_format_response(self): 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 dead code guarded by ``NoSitesError``: that + check in ``_querying`` fires only on a body starting "No sites/data", + which is what the live service happens to send today -- a comment-only + RDB reaches the guarded line instead. + """ + no_peaks_rdb = ( + "# //Output-Format: RDB\n" + "# //Response-Status: OK\n" + "# //Response-Message: No sites found matching all criteria\n" + ) + # Mirror get_discharge_peaks: raw read_rdb with the NWIS dtype hints, + # then the peaks-specific format_response. + df = read_rdb(no_peaks_rdb, dtypes=_NWIS_RDB_DTYPES) + df = format_response(df, service="peaks") + assert isinstance(df, pd.DataFrame) + assert df.empty + + def test_malformed_peaks_frame_still_raises(self): + """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 TestGetRecordDispatch: """``get_record`` is a router; each service must reach its own getter.