feat: add a unified county argument, like state - #433
Draft
thodson-usgs wants to merge 5 commits into
Draft
thodson-usgs wants to merge 5 commits into
thodson-usgs wants to merge 5 commits into
Conversation
thodson-usgs
added a commit
to thodson-usgs/dataretrieval-python
that referenced
this pull request
Oct 2, 2026
Simplify review of DOI-USGS#433: - apply_county deduplicates the counties before calling render, so the three renderers no longer each repeat list(dict.fromkeys(...)). - Renderers return lists unconditionally. The single-value collapse was unneeded: the request builder already sends a one-element list as a plain GET parameter (checked live: same URLs and rows). - The multi-state-with-filter conflict raises through _validation.reject_together, in the package's shared wording. - The Connecticut hint is keyed on the parsed code rather than a second, looser parse of the raw value. - The location getters' county docstrings name the US:55:025 form as the others do.
codes.states named FIPS 78 "US Virgin Islands". Water Data and NGWMN both filter state_name on "Virgin Islands" and match exactly, so state="VI" returned no rows from either service: 0 rather than 1,092 monitoring locations, and 0 rather than 3 NGWMN sites. The Census name is still accepted as input. The territory tests are consolidated to one row per territory, each checking that its name, postal code, and FIPS code resolve to one another; the encodings themselves (case, US: prefix) are Test_to_state's.
county accepts a five-digit FIPS code, the statistics service's
US:SS:CCC, or a name or three-digit code with its state, and sends each
service what it filters on: Water Data state_code + county_code,
statistics county_code=US:SS:CCC, NGWMN state_name + county_name, NWDC
countyCd. With county, state qualifies the counties instead of filtering
on its own. Scoped as state is: the location and statistics getters,
ngwmn.get_sites, and nwdc.get_wateruse; not WQP, NWIS, or Samples, which
have no state argument either, and not get_time_series_metadata or
ngwmn.get_providers, which have no county field.
The table is the Water Data counties collection (3,239 counties in the
states and territories codes.states covers), in its own data module so
the doubled-word hook can skip it ("Walla Walla County"). Every entry
round-trips through every input form, and a live test compares the
table with the collection.
A county code repeats across states, so counties in several states go
to the Water Data location collections as a cql-text filter OR-ing
(state_code, county_code) pairs, which the planner splits at top-level
OR: all 174 Wisconsin and Illinois counties returned the same 5,136
stream sites as state=[WI, IL], in 2 requests. NGWMN's edge refuses
cql-text filters (403), so get_sites takes one state per call.
NGWMN names Anchorage differently and still files Connecticut sites
under the counties the state replaced with planning regions in 2022;
the first is aliased, the second raises with a county_name remedy, and
a live test fails if either changes.
Fixes ngwmn.get_sites(county_name=[...]) returning no rows: NGWMN
matches nothing for a comma-joined county_name, so multi-value sites
filters are now POSTed as CQL2 (checked live for state_name,
monitoring_location_id, agency_code, and county_name).
state and now county are package-named arguments for domain concepts that ADR 0013 otherwise leaves to each service's spelling. Review of DOI-USGS#422 read state as hiding the APIs' own parameters, and nothing recorded why it did not contradict the rule. ADR 0013 gains a clause stating the four conditions both meet: the native parameters stay, the conversion is an exact table kept against the service's reference collection, combining the two raises, and the docstring names what the argument is sent as. CONTEXT.md defines State and County as domain terms with each service's spelling, and Unified argument as a core term.
Simplify review of DOI-USGS#433: - apply_county deduplicates the counties before calling render, so the three renderers no longer each repeat list(dict.fromkeys(...)). - Renderers return lists unconditionally. The single-value collapse was unneeded: the request builder already sends a one-element list as a plain GET parameter (checked live: same URLs and rows). - The multi-state-with-filter conflict raises through _validation.reject_together, in the package's shared wording. - The Connecticut hint is keyed on the parsed code rather than a second, looser parse of the raw value. - The location getters' county docstrings name the US:55:025 form as the others do.
ADR 0013's unified-argument clause requires each conversion table to be checked live against the service's reference collection. The county table is (counties_test.py); this does the same for codes.states, whose names Water Data and NGWMN match exactly. It would have caught the Virgin Islands name fixed in DOI-USGS#432.
thodson-usgs
force-pushed
the
feat/county-argument
branch
from
October 2, 2026 01:09
ba2115e to
c77c9b6
Compare
This branch has not been deployed
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
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.
TL;DR: Adds a
countyargument that works likestate. It accepts a FIPS code, or a name with its state, and sends each service the fields that service filters on. I checked every conversion against the live services. Scope followsstate: Water Data's location and statistics getters,ngwmn.get_sites, andnwdc.get_wateruse, but not WQP, NWIS, or Samples. ADR 0013 andCONTEXT.mdnow record the rule both arguments follow.Stacked on #432 (Virgin Islands name fix): its commit
e3c3c335is the first one here. Merge #432 first; review the commits after it.Changes
codes.counties(new; exposesto_countyandapply_county). The table holds 3,239 counties and county equivalents. It is a snapshot of the Water Datacountiescollection for the states and territories thatcodes.statescovers, and a live test compares the two. Accepted inputs:"55025",55025, or"US:55:025";state:"025","Dane County","dane", or"St Louis County"(case and periods don't matter, and the designation such as County or Parish is optional).Getters. In every getter below,
statenames the state the counties are in whencountyis given, instead of adding its own filter. Passingcountytogether with the getter's own state or county parameter raisesValueError.waterdata.get_monitoring_locations,get_combined_metadatastate_code+county_code; counties in several states become afilterof OR'd pairswaterdata.get_stats_por,get_stats_date_rangecounty_code=US:55:025(repeated)ngwmn.get_sitesstate_name+county_namenwdc.get_waterusecountyCd:55025(the existingcountynow accepts every form above)Bug fix (NGWMN):
get_sites(county_name=[...])with more than one name returned no rows, because NGWMN matches nothing for a comma-joinedcounty_name. Multi-valuesitesfilters are now POSTed as CQL2.Docs: ADR 0013 gains a clause allowing a unified argument (an argument dataretrieval names for a concept the services spell differently) under four conditions:
CONTEXT.mddefines State and County as domain terms and Unified argument as a core term.Design choices for the maintainer
stateraises. A bare name that matches two counties in one state also raises and lists both. This happens 12 times, e.g. Fairfax County and Fairfax City in Virginia.(state_code='55' AND county_code='025') OR …. Separatestate_codeandcounty_codelists would also match code 031 in Wisconsin. The request planner already splits a filter at its top-levelOR. The catch is thatcountyfrom more than one state can't be combined with your ownfilter, and the call raises instead, with a remedy.get_sites(county="09110")therefore raises and points tocounty_name="Hartford County". NGWMN'sMunicipality of Anchorageis handled with a one-line alias. A live test fails if NGWMN changes either.get_wateruse(state=..., county=...)used to raise for naming two locations. Nowstatequalifies the county. An unknown county, such as Connecticut's retired09003, raises locally instead of returning HTTP 400._county_table.py, so that the doubled-word hook can skip it ("Walla Walla County").Verification
US:form, and integer. Each bare name resolves unless it's one of the 12 ambiguous ones.get_monitoring_locationsreturns only rows with the requestedstate_code,county_code, andcounty_name.ngwmn.get_sitesreturns only the requestedstate_name+county_name, or raises for Connecticut.state="AK"does the same today.county=with all 174 Wisconsin and Illinois counties returns the same 5,136 stream sites asstate=["WI", "IL"], in 2 requests.countiescollection, NGWMN's county names, and the state table against thestatescollection (moved here from fix(codes): name the Virgin Islands as the services do #432, because ADR 0013's new clause cites it).--strict, import-linter, xenon, and complexipy pass, and so do the pre-commit hooks. The Sphinx build (notebooks not executed) adds no warnings.