Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitlab/issue_templates/reviewer_checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Background information for reviewers [here](https://www.usgs.gov/products/softwa

- [ ] No proprietary code is included
- [ ] No Personally Identifiable Information (PII) is included
- [ ] No other sensitive information such as data base passwords are included
- [ ] No other sensitive information such as database passwords are included

### General checks

Expand Down
2 changes: 1 addition & 1 deletion .gitlab/merge_request_templates/reviewer_checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Background information for reviewers [here](https://www.usgs.gov/products/softwa

- [ ] No proprietary code is included
- [ ] No Personally Identifiable Information (PII) is included
- [ ] No other sensitive information such as data base passwords are included
- [ ] No other sensitive information such as database passwords are included

### General checks

Expand Down
30 changes: 3 additions & 27 deletions .importlinter
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ layers =
; The deprecated ``wateruse`` alias re-exports ``nwdc``, so it sits above the
; adapters rather than beside them. A compatibility facade may depend on the
; adapter it forwards to; nothing may depend on the facade.
wateruse
ngwmn | nldi | nwdc | nwis | streamstats | waterdata | wqp
; The removed ``nwis`` stub only raises ``ImportError``, so nothing may import it.
nwis | wateruse
ngwmn | nldi | nwdc | streamstats | waterdata | wqp
ogc
utils
_querying
Expand Down Expand Up @@ -122,31 +123,6 @@ forbidden_modules =
dataretrieval._querying
dataretrieval.utils

[importlinter:contract:nwis-quarantine]
name = Deprecated NWIS has no dependents (ADR 0005)
type = forbidden
source_modules =
dataretrieval._querying
dataretrieval.codes
dataretrieval.combining
dataretrieval.credentials
dataretrieval.exceptions
dataretrieval.interruptions
dataretrieval.ngwmn
dataretrieval.nldi
dataretrieval.ogc
dataretrieval.progress
dataretrieval.rdb
dataretrieval.streamstats
dataretrieval.transport
dataretrieval.utils
dataretrieval.nwdc
dataretrieval.waterdata
dataretrieval.wateruse
dataretrieval.wqp
forbidden_modules =
dataretrieval.nwis

[importlinter:contract:waterdata-families]
name = Water Data collection families do not reach through each other (ADR 0007)
type = independence
Expand Down
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ repos:
# Cognitive-complexity ratchet, the counterpart to xenon's cyclomatic one.
# The two measure different things: xenon counts branches, complexipy counts
# how hard the control flow is to hold in your head, so nesting and early
# exits weigh differently. 25 is the package's current maximum
# (``nwis._read_json``), so this holds the line rather than demanding a
# refactor. Unlike xenon this one is per-function, so scanning only the
# changed files gives the same verdict CI gives for the whole package.
# exits weigh differently. The ceiling is ``max-complexity-allowed`` in
# ``[tool.complexipy]`` (pyproject.toml), which holds the line rather than
# demanding a refactor. Unlike xenon this one is per-function, so scanning
# only the changed files gives the same verdict CI gives for the whole package.
- repo: https://github.com/rohaquinlop/complexipy-pre-commit
rev: v6.2.0
hooks:
- id: complexipy
files: ^dataretrieval/.*\.py$

# Dependency-direction contracts (ADR 0003, 0005, 0006, 0007). Complements
# Dependency-direction contracts (ADR 0003, 0006, 0007). Complements
# tests/architecture_test.py, which enforces named pairwise claims by parsing
# the AST; this checks the whole layer stack over the *transitive* graph, so
# a service reaching transport through OGC fails here. Runs in about a
Expand Down
33 changes: 4 additions & 29 deletions .pyscn-known-clones.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,11 @@
]
},
"observed": {
"total_fragments": 351,
"cloned_fragments": 13,
"groups": 5
"total_fragments": 349,
"cloned_fragments": 11,
"groups": 4
},
"groups": [
{
"members": [
"dataretrieval/_validation.py::reject_together",
"dataretrieval/_validation.py::require_any_of"
],
"similarity": 0.85,
"clone_type": 4,
"rationale": "Shared argument checks (AGENTS.md, Error messages): each returns early when the arguments are valid and otherwise raises one message from the same parts. Each check owns its wording, so a common helper would only move the message apart from the check that states it."
},
{
"members": [
"dataretrieval/nwdc.py::_validate_county",
Expand Down Expand Up @@ -87,21 +78,5 @@
"clone_type": 4,
"rationale": "Two Water Data getters matched as a semantic (type-4) clone at 0.672, just above the 0.65 threshold. Expect this pair to appear and disappear between runs as either getter is edited; that churn is not a new clone."
}
],
"sub_threshold_accepted": {
"_comment": "Families that pyscn 1.29.0 did NOT report at its defaults (they sat under its min_lines=10 / min_nodes=20) but that a deliberate scan at min_nodes=8 found on 2026-08-09. Not re-scanned with 1.32.4, whose min_lines is 8. Recorded so they are not rediscovered and re-litigated. They are NOT counted in `observed` above.",
"groups": [
{
"members": [
"dataretrieval/nwis.py::get_discharge_measurements",
"dataretrieval/nwis.py::get_gwlevels",
"dataretrieval/nwis.py::get_pmcodes",
"dataretrieval/nwis.py::get_qwdata",
"dataretrieval/nwis.py::get_water_use"
],
"similarity": 0.83,
"rationale": "Deprecated NWIS getters (ADR 0005). The surface is frozen pending removal, so collapsing it buys nothing and churns a public API users are being migrated off."
}
]
}
]
}
14 changes: 7 additions & 7 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
and takes one spelling everywhere *including identifiers*, so a name in the
code that conflicts with it is a defect. A **domain** term -- *monitoring
location*, *collection* -- is fixed for prose only: each adapter keeps its
own service's spelling in its parameters, so `nwis.get_record(service="dv")`
and WQP's `Station` are that service's vocabulary rather than drift. Each
own service's spelling in its parameters, so WQP's `siteid` and `Station`
are that service's vocabulary rather than drift. Each
entry states which kind it is.
- Architectural decisions and their rationale: `docs/source/architecture/decisions/`
(ADRs, referenced by number throughout the code and by `.importlinter`).
Expand All @@ -21,8 +21,8 @@ Use `ls`/`grep` for the file list; what follows is the placement logic, so you
can predict where a thing is defined.

- `dataretrieval/` — the public surface is one *adapter* module per service,
named for the service (`nldi`, `nwdc`, `ngwmn`, `streamstats`, `wqp`, and
legacy `nwis`); each owns that service's URLs, parameters, and response
named for the service (`nldi`, `nwdc`, `ngwmn`, `streamstats`, and `wqp`);
each owns that service's URLs, parameters, and response
quirks. `waterdata/` is the one adapter large enough to be a package, split by
collection family; its `api.py` is a compatibility facade holding no logic.
Everything else in the package is shared code the adapters depend on
Expand Down Expand Up @@ -112,8 +112,8 @@ raise states the problem and then the action that fixes it, in that order.
- Don't invent a phrasing for a check that recurs. `dataretrieval/_validation.py`
owns the wording for the shared cases — bad value in a closed vocabulary
(`require_one_of`), missing argument (`require_argument`), incomplete group
(`require_together`), no filter at all (`require_any_of`), and conflicting
arguments (`require_exactly_one`, `reject_together`). Use one before
(`require_together`), and conflicting arguments
(`require_exactly_one`, `reject_together`). Use one before
hand-writing a message. A service-specific pointer is not a reason to
hand-write: every check takes a `remedy=` for the action it cannot derive.
Every check raises `ValueError` -- one class for a bad argument value, so a
Expand Down Expand Up @@ -146,7 +146,7 @@ raise states the problem and then the action that fixes it, in that order.
- Public getters return `(DataFrame, metadata)`.
- `dataretrieval/__init__.py` imports the service modules by name and lists them
in `__all__`; it does not star-import them, so a getter is accessed through its
module (`dataretrieval.nwis.get_record`), never from the top level. `nldi` is
module (`dataretrieval.wqp.get_results`), never from the top level. `nldi` is
deliberately absent — it needs `geopandas` at import time, so it is imported on
demand. `dataretrieval/waterdata/__init__.py` controls Water Data exports via
`__all__`.
Expand Down
30 changes: 15 additions & 15 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ the reason is not part of the term.

**Plan** — An enumeration of a query's chunks: how many there are, and what each
one is. A plan describes how a query divides; it does not execute. Computing a plan
is protocol-specific — a byte budget, a per-location rule — while executing one
is protocol-specific — a byte budget, a per-location rule — whereas executing one
is not, which is why the two are kept in separate modules.

**Fan-out** — Executing a query's chunks concurrently. Chunking is how the work
Expand All @@ -66,7 +66,7 @@ rate limit, a service error, a timeout. Distinguished from a **deterministic
failure**, which would fail identically every time — an unresolvable hostname,
an unsupported scheme, a malformed request. Only transient failures are
retried, and only transient failures produce a resumable interruption. Both
answers follow from one judgement about what a failure means, and must be
answers follow from one judgment about what a failure means, and must be
consistent.

**Stall timeout** — How long a call may receive nothing at all before retrying
Expand Down Expand Up @@ -106,8 +106,12 @@ what the service offers.
**NLDI** — The Network Linked Data Index, which navigates the hydrologic network
from an origin to connected features, flowlines, or basins.

**NWIS** — The legacy USGS waterservices interface. Deprecated: it is retained
for compatibility and should receive no new work.
**NWIS** — The legacy USGS waterservices interface. Retired: the
`dataretrieval.nwis` adapter has been removed (ADR 0005), so
`import dataretrieval.nwis` raises `ImportError` naming the replacements
and the last release that includes it (1.4.0). The term is kept
because "NWIS" survives as a data-provider name and data value in other services
(WQP `providers=NWIS`, the USGS NWIS time-zone code list) and in this history.

**StreamStats** — Basin characteristics and delineation for a point on a stream.

Expand All @@ -121,14 +125,10 @@ collections. The distinction matters because the OGC code is shared: the
same code path retrieves a Water Data collection and an NGWMN one, and only the
service differs.

*Domain term.* Water Data and NGWMN say `collection`; legacy NWIS says
`service=` (`'dv'`, `'peaks'`); WQP names its collections as profiles
(`Result`, `Station`, `Activity`). Those parameters are frozen public surface
and keep their spelling. Prose says *collection*, including prose about the
adapters that spell it otherwise — with one exception: deprecated NWIS keeps
`service` in its docstrings as well as its parameters. Describing a parameter
in a term its own module never uses helps nobody, and a module being retired is
not where new vocabulary should be introduced.
*Domain term.* Water Data and NGWMN say `collection`; WQP names its collections
as profiles (`Result`, `Station`, `Activity`). Those parameters are frozen
public surface and keep their spelling. Prose says *collection*, including prose
about the adapters that spell it otherwise.

**Collection family** — A group of collections sharing a shape and therefore a
getter signature. Their getters deliberately resemble one another; the
Expand All @@ -137,8 +137,8 @@ resemblance is the public contract, not duplication to be removed.
**Monitoring location** — A place where measurements are recorded.

*Domain term.* The services differ, and each adapter keeps its own service's
spelling in its parameters: NWIS `site_no` and `sites=`, WQP `Station` and
`siteid`, Water Data `monitoring_location_id`, NGWMN's `sites` collection. Where
spelling in its parameters: WQP `Station` and `siteid`, Water Data
`monitoring_location_id`, NGWMN's `sites` collection. Where
a service names a thing `site-types` or `site_type_code`, that is its vocabulary
and is reproduced rather than translated. Prose that is not about one particular
adapter says *monitoring location*.
Expand Down Expand Up @@ -264,7 +264,7 @@ adapter supplies one and the shared code reads it, which is how protocol code
stays free of service names.

**Single-shot adapter** — An adapter whose query is always exactly one request:
WQP, NLDI, StreamStats, and deprecated NWIS. Nothing divides and nothing
WQP, NLDI, and StreamStats. Nothing divides and nothing
distributes, so `concurrency` and `parallel_chunks` are not part of its
vocabulary. NWDC is not one: its query fans out per location even though it
never chunks by bytes.
Expand Down
10 changes: 4 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ restructuring.

`lint-imports` checks the dependency contracts declared in
[`.importlinter`](.importlinter) against the *transitive* import graph: the layer
stack, which modules may consume OGC, NGWMN's facade-only seam, the NWIS
quarantine, and collection-family independence.
stack, which modules may consume OGC, NGWMN's facade-only seam, and
collection-family independence.

**That file is the only place dependency direction is enforced.** These rules
were once asserted a second time in `tests/architecture_test.py` by hand-parsing
Expand Down Expand Up @@ -246,16 +246,14 @@ link checking.
* The public interface should emphasize functions over classes; classes can and
should be used internally and in tests.
* Group public download functions by service. For example, modern Water
Data functions belong in `dataretrieval.waterdata`; legacy NWIS functions
remain quarantined in `dataretrieval.nwis` during deprecation.
Data functions belong in `dataretrieval.waterdata`.
* Treat a change to a service's documented return shape or metadata type as a
public compatibility change; update contract tests and architecture
documentation and follow the deprecation process where required.
* Preserve the dependency direction documented in
[`docs/source/architecture`](docs/source/architecture/index.rst): public
facades depend on service/protocol adapters, which depend on stable shared
policy and infrastructure. Shared OGC code must not import service adapters,
and modern modules must not depend on legacy NWIS.
policy and infrastructure. Shared OGC code must not import service adapters.
* Treat underscore-prefixed helpers as implementation details. Existing
cross-package uses are documented variances, not extension points for new
code.
Expand Down
Loading
Loading