Skip to content

fix(nwis): forward get_record state, fix major-filter validation, reject bad format#310

Draft
thodson-usgs wants to merge 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix/nwis-record-filter-handling
Draft

fix(nwis): forward get_record state, fix major-filter validation, reject bad format#310
thodson-usgs wants to merge 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix/nwis-record-filter-handling

Conversation

@thodson-usgs
Copy link
Copy Markdown
Collaborator

@thodson-usgs thodson-usgs commented May 30, 2026

Summary

Several fixes in the (deprecated) nwis module, all surfaced by the package review.

1. get_record(state=…) was a dead parameter — accepted and documented, but never forwarded. get_record(state="OH", service="site") silently ignored it and, with no sites, failed with a confusing "Bad Request." Now forwarded as the NWIS stateCd major filter.

2. The "must specify a major filter" validation was defeated. The getters injected the filter key unconditionally (e.g. kwargs["sites"] = kwargs.pop("sites", sites) set sites=None when absent), so the membership-based guards in query_waterservices / query_waterdata always passed and a filterless request went out as a confusing "Bad Request" instead of the intended TypeError. Fixed once at the chokepoints: the major-filter guards now require a filter that is present and non-None. (utils.query already strips None-valued params before the request, so this is purely a validation change — no per-getter pre-filtering needed.)

3. format= collided / was silently overridden. get_dv / get_iv / get_discharge_peaks / get_stats each parse a fixed response body but passed format= explicitly alongside **kwargs, so e.g. get_dv(sites=…, format="rdb") raised TypeError: got multiple values for 'format'. A caller-supplied non-native format is now rejected with a clear ValueError via a shared _reject_unexpected_format helper (json for dv/iv, rdb for peaks/stats).

4. get_ratings is per-site. Called without a site it issued a request that returned an unhelpful error page; it now fails fast with a clear TypeError (also covers get_record(service="ratings")).

Verification (no network)

get_dv()                       -> TypeError: ...must specify a major filter   (was: Bad Request)
get_dv(sites=…, format="rdb")  -> ValueError: get_dv returns JSON ...         (was: multiple values for 'format')
get_record(state="OH", ...)    -> forwards stateCd="OH"                       (was: silently dropped)
get_ratings()                  -> TypeError: ...requires a `site`             (was: Bad Request page)

Regression tests added for each. Full nwis suite passes; ruff and mypy --strict clean.

🤖 Generated with Claude Code

…ect bad format

Several fixes in the deprecated `nwis` module, surfaced by the package review.

- `get_record(state=…)` was accepted and documented but never forwarded. It now
  reaches the request as the NWIS `stateCd` major filter; previously it was
  silently dropped, producing a confusing "Bad Request" when used without
  `sites`.

- The "must specify a major filter" validation was defeated: callers injected a
  filter key unconditionally (e.g. `kwargs["sites"] = kwargs.pop("sites", sites)`
  set the key to None when absent), so the membership-based guards in
  `query_waterservices`/`query_waterdata` always passed and a filterless request
  went out as a confusing "Bad Request" instead of the intended TypeError. The
  guards now require a filter that is present *and* non-None, rejecting an unset
  filter at the chokepoint — no per-getter pre-filtering needed. (`utils.query`
  already strips None-valued params, so this is purely a validation change.)

- `get_dv`/`get_iv`/`get_discharge_peaks`/`get_stats` each parse a fixed response
  body but passed `format=` explicitly alongside `**kwargs`, so e.g.
  `get_dv(sites=…, format="rdb")` raised "multiple values for 'format'". A
  caller-supplied non-native `format` is now rejected with a clear ValueError via
  the shared `_reject_unexpected_format` helper (json for dv/iv, rdb for
  peaks/stats).

- `get_ratings` is per-site; called without one it issued a request returning an
  unhelpful error page. It now fails fast with a clear TypeError (also covering
  `get_record(service="ratings")`).

Adds regression tests for each. Full nwis suite passes; ruff and mypy --strict
clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@thodson-usgs thodson-usgs force-pushed the fix/nwis-record-filter-handling branch from b040a74 to f2a2503 Compare June 3, 2026 01:06
@thodson-usgs thodson-usgs changed the title fix(nwis): forward get_record state, restore major-filter validation, clarify format fix(nwis): forward get_record state, fix major-filter validation, reject bad format Jun 3, 2026
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.

2 participants