Skip to content

fix(nwis): name the peak date format instead of making pandas guess - #403

Open
thodson-usgs wants to merge 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix/peaks-datetime-format
Open

fix(nwis): name the peak date format instead of making pandas guess#403
thodson-usgs wants to merge 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix/peaks-datetime-format

Conversation

@thodson-usgs

Copy link
Copy Markdown
Collaborator

Why

preformat_peaks_response calls 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, falls back to per-element dateutil parsing, and says so:

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.

Found while reviewing #395 against the glossary; it is a separate concern from
that review, so it is a separate PR.

The fix

The format was never actually ambiguous. The RDB header types peak_dt as
10d — a ten-character date — and any time is carried separately in peak_tm:

agency_cd  site_no   peak_dt  peak_tm  peak_va  ...
5s         15s       10d      6s       8s

So %Y-%m-%d is the only shape the column takes, and naming it stops pandas
guessing.

Verified identical, not assumed

Against the real tests/data/waterservices_peaks.txt column plus zero-filled,
blank, malformed and time-bearing values:

input shape warns today warns after results identical
all YYYY-MM-DD no no
zero-filled + parseable yes no
all zero-filled yes no
carrying a time component no no
malformed no no

The warning fires only on the case #395 made normal. A value carrying a time
coerces to NaT either way, so nothing that parsed before stops parsing.

The test can fail

test_preformat_peaks_response_is_quiet_on_partly_dated_peaks asserts silence
with warnings.simplefilter("error"). It was run against the unfixed
function first and confirmed to fail there:

FAILED tests/nwis_test.py::test_preformat_peaks_response_is_quiet_on_partly_dated_peaks
  UserWarning: Could not infer format ...

1140 tests pass, mypy --strict clean, all hooks green.

Scope

Independent of #400 and #402. Touches nwis.py, one new test, and a NEWS entry.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD

`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 DOI-USGS#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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JAEQqs7XzQHGQQi2KakuXD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant