From c5980629d1e1cb15fd52c6c3f8350d2aea0c58a1 Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Tue, 1 Sep 2026 12:57:10 -0500 Subject: [PATCH] fix(nwis): name the peak date format instead of making pandas guess `preformat_peaks_response` called `pd.to_datetime(df["peak_dt"], errors="coerce")`. On a column mixing zero-filled and parseable dates pandas cannot find one format that fits both, so it falls back to per-element `dateutil` parsing and says so on stderr: UserWarning: Could not infer format, so each element will be parsed individually, falling back to `dateutil`. That mix was rare until #395, which stopped dropping peaks whose date NWIS only partly knows. Keeping them made the mix the normal case for any long historical record, so the warning now fires on ordinary `get_discharge_peaks` calls -- library noise on stderr for behaviour the library intends. The format is not actually ambiguous. The RDB header types `peak_dt` as `10d`, a ten-character date, and any time is carried separately in `peak_tm`, so `%Y-%m-%d` is the only shape the column takes. Verified identical, not assumed. Against the real `tests/data/waterservices_peaks.txt` column plus zero-filled, blank, malformed, and time-bearing values, `errors="coerce"` with and without `format="%Y-%m-%d"` produce the same result on every row; only the warning differs. A value carrying a time coerces to `NaT` either way. The regression test asserts silence via `simplefilter("error")`, and was confirmed to fail against the unfixed function before being kept. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD --- NEWS.md | 2 ++ dataretrieval/nwis.py | 6 +++++- tests/nwis_test.py | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 5aa293f4..cf0b0620 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ **09/01/2026:** **Announcement:** We at USGS Water Data for the Nation want your feedback! Tell us how we're doing by taking our quick [survey](https://usgswaterresources.gov1.qualtrics.com/jfe/form/SV_07gX8G1DeOtVrH8), available through September 2026. +**09/01/2026:** **Bug fix:** `nwis.get_discharge_peaks` and `nwis.get_record(service='peaks')` printed a pandas `UserWarning` ("Could not infer format, so each element will be parsed individually") on any record holding a peak whose date NWIS only partly knows. Those became ordinary in a long historical record once such peaks stopped being dropped, so the warning fired on routine calls. `peak_dt` is typed `10d` by the RDB header and any time is carried in `peak_tm`, so the format is fixed and is now named. Dates parse identically either way; only the noise is gone. + **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. diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py index 701f7a22..17f0ef9d 100644 --- a/dataretrieval/nwis.py +++ b/dataretrieval/nwis.py @@ -236,7 +236,11 @@ def preformat_peaks_response(df: pd.DataFrame) -> pd.DataFrame: # still raise. return df - df["datetime"] = pd.to_datetime(df["peak_dt"], errors="coerce") + # RDB types ``peak_dt`` as ``10d`` and carries any time in ``peak_tm``, so + # the format is fixed. Naming it also keeps pandas from warning that it + # could not infer one, which zero-filled dates otherwise provoke on every + # long historical record. + df["datetime"] = pd.to_datetime(df["peak_dt"], format="%Y-%m-%d", errors="coerce") return df diff --git a/tests/nwis_test.py b/tests/nwis_test.py index 78e5b98b..0b92a244 100644 --- a/tests/nwis_test.py +++ b/tests/nwis_test.py @@ -128,6 +128,26 @@ def test_preformat_peaks_response_preserves_peak_dt(): assert df["peak_dt"].iloc[0] == "1858-00-00" +def test_preformat_peaks_response_is_quiet_on_partly_dated_peaks(): + """A frame mixing zero-filled and parseable dates must not warn. + + Left to infer a format, pandas cannot find one that fits both and says so + on stderr. Partly dated peaks are normal in a long historical record, so + that warning would fire on ordinary calls; the dates parse identically + either way, only the noise differs. + """ + df = pd.DataFrame( + {"peak_dt": ["1858-00-00", "1900-01-02"], "peak_va": [847000, 12300]} + ) + + with warnings.catch_warnings(): + warnings.simplefilter("error") + out = preformat_peaks_response(df) + + assert pd.isna(out["datetime"].iloc[0]) + assert out["datetime"].iloc[1] == pd.Timestamp("1900-01-02") + + 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