fix(nwis): handle empty peaks response instead of raising KeyError#344
Open
arpitjain099 wants to merge 2 commits into
Open
fix(nwis): handle empty peaks response instead of raising KeyError#344arpitjain099 wants to merge 2 commits into
arpitjain099 wants to merge 2 commits into
Conversation
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 DOI-USGS#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 DOI-USGS#171 coverage.
Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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 <thodson@usgs.gov> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Calling
nwis.get_discharge_peaksorget_record(service="peaks")for a site with no annual-peak data raisesKeyError('peak_dt')instead of returning an empty result.When peaks has no data, the RDB body is comment lines only, so
read_rdbparses it to a column-less empty DataFrame.format_responserunspreformat_peaks_responsebefore its own "datetime not in columns" empty-frame check, and that function's first line popspeak_dt, which blows up on the empty frame.This is the same empty-result behavior that #171 fixed for the other services; peaks slipped through because it gets preformatted first. The fix returns the frame unchanged when
peak_dtis absent, so the existing empty-frame path informat_responsetakes over and callers can checkdf.emptyrather than catching an exception.Added a regression test in
TestReadRdbnext to the existing #171 coverage. It fails on main withKeyError('peak_dt')and passes with the change.ruff checkandruff format --checkare clean on both touched files.I work on supply-chain and data-tooling robustness and hit this while looking at the empty-response paths. Happy to adjust if you would rather guard this inside
format_responseinstead.