Skip to content

Check getter parameter docs against the service's queryables #379

Description

@thodson-usgs

Problem Statement

A caller who runs help(waterdata.get_daily) — or reads the same getter in the
Sphinx reference — sees a hand-written description of parameter_code, time,
last_modified, and every other typed keyword argument. The Water Data service
publishes its own description of each of those properties in the collection's
queryables document. Nothing checks that the two agree.

So the caller cannot tell, from the documentation alone:

  • whether a description is the service's wording or ours,
  • how long ago it was last true,
  • whether a queryable the service has since documented differently is now
    described wrongly here.

The drift is invisible in both directions. Today's committed queryables snapshot
tracks queryable names only, so a renamed or added queryable is caught while
a revised description passes silently. And because collection families
deliberately share a getter signature, the same prose is maintained in many
places at once: across the package there are 453 numpydoc parameter blocks that
resolve to only 311 distinct descriptions — roughly 37 KB duplicated verbatim,
with time_series.py and measurements.py running about 80% and 76% docstring
by bytes. max_rows is written out 11 times, last_modified 7 times at 744
bytes each. A revision to one is a revision to all of them, by hand, with no
check that the sweep was complete.

The sibling R package has the guarantee we lack: its generated documentation
interpolates the live schema, so its parameter descriptions cannot drift from
the service. It buys that with a documentation-generation step. We want the
guarantee without the step.

Solution

Keep the hand-written docstrings, and make the service the checked authority
rather than the runtime source.

Docstrings stay plain literals in the source. help(), Sphinx, IDE hovers, and
mypy keep working exactly as they do now; nothing rewrites __doc__, nothing
is fetched at import time, and no getter grows a decorator. What changes is that
a disagreement between our prose and the service's prose becomes a failing
check instead of a thing a reviewer might notice.

Three parts:

  1. The snapshot learns descriptions. The committed queryables snapshot
    already records, per collection, which queryables exist. It grows to record
    each queryable's type, title, and description as well — the same three
    fields the queryables getter already surfaces in its frame.

  2. A merge gate compares docstrings to the snapshot, offline. For every
    Water Data getter, every typed keyword argument that corresponds to a
    queryable of that getter's collection must have a documented description that
    agrees with the snapshot. Runs on every push with no network, like the rest
    of the suite.

  3. A weekly advisory check compares the snapshot to the live service. A
    snapshot can only catch drift between our docs and our snapshot; it can never
    catch drift between the snapshot and the service. The existing weekly
    advisory sweep gains that comparison and reports what moved, so the snapshot
    ages visibly instead of quietly.

Refreshing is one command that rewrites the snapshot and prints which
descriptions changed, so the follow-up docstring edit is a bounded, reviewable
diff rather than an archaeology exercise.

User Stories

  1. As a hydrologist reading help(waterdata.get_daily), I want each parameter
    description to match what the Water Data service says about that queryable,
    so that I do not build a query on a description the service has since
    revised.
  2. As a hydrologist, I want to know that the descriptions I am reading were
    verified against the service, so that I can trust them without opening the
    API's own documentation in another tab.
  3. As a hydrologist comparing get_daily and get_continuous, I want the
    shared queryables to be described identically, so that a wording difference
    reads as a real difference in the data rather than as an editing accident.
  4. As a scientist moving from the R package to this one, I want the parameter
    documentation to be as authoritative as the R package's schema-interpolated
    docs, so that switching languages does not cost me confidence in the docs.
  5. As a maintainer, I want a failing check when a getter's documented parameter
    description disagrees with the service, so that stale prose is caught at
    merge rather than in a user's issue report.
  6. As a maintainer, I want that check to run offline against a committed
    snapshot, so that a push does not depend on USGS uptime or spend a rate
    limit.
  7. As a maintainer, I want a separate scheduled check against the live service,
    so that drift between the snapshot and reality is reported even though the
    merge gate cannot see it.
  8. As a maintainer, I want the scheduled check to be advisory and never fail the
    build, so that an upstream wording change does not block unrelated work.
  9. As a maintainer, I want one command that refreshes the snapshot and reports
    which descriptions changed, so that responding to upstream drift is a
    bounded edit.
  10. As a maintainer, I want the refresh output to name the getters affected by
    each changed queryable, so that I know every place the prose has to be
    updated without grepping.
  11. As a maintainer, I want the check to cover every Water Data collection the
    package exposes a getter for, so that a collection cannot be quietly
    exempted.
  12. As a maintainer, I want a new collection family or getter to be covered the
    day it is added, so that coverage does not depend on someone remembering to
    extend a list.
  13. As a maintainer, I want a queryable that the service documents but the
    getter does not expose as a typed argument to be reported, so that I can
    decide whether to promote it rather than never hearing about it.
  14. As a maintainer, I want a typed argument that is not a service queryable
    (convert_type, max_rows, limit, ssl_check) to be recognised as
    package-owned and exempt, so that the check does not demand a service
    description for a thing the service knows nothing about.
  15. As a maintainer, I want those package-owned arguments to be declared in one
    place, so that their descriptions are consistent across getters without
    being retyped 11 times.
  16. As a maintainer, I want the comparison to tolerate our own added prose — a
    usage note, an example, a Python-specific caveat — so that the check does
    not force us to delete the parts of a description that make it useful in
    Python.
  17. As a maintainer, I want the comparison to normalise trivial differences
    (trailing newlines, wrapping, whitespace runs), so that a reflow does not
    read as drift.
  18. As a maintainer, I want the failure message to print the service's text and
    ours side by side, so that the fix is obvious from the CI log without
    reproducing locally.
  19. As a maintainer, I want the failure message to name the collection, the
    queryable, and the getter, so that I can go straight to the edit.
  20. As a contributor adding a getter for a new collection, I want the check to
    tell me which parameter descriptions I still owe, so that review is not
    where that is discovered.
  21. As a contributor, I want the check to run in my pre-commit hooks the same
    way it runs in CI, so that a clean local run means CI agrees.
  22. As a contributor, I want the check to be fast and offline, so that it does
    not make the suite slower to run or flakier.
  23. As a maintainer, I want this to add no third-party runtime dependency to the
    package, so that installing dataretrieval costs no more than it does now.
  24. As a maintainer, I want no new module inside the shipped package if the work
    can live in test support, so that the declared dependency layers and the
    exhaustive layer contract stay untouched.
  25. As a maintainer, I want docstrings to remain plain literals rather than
    generated or rewritten at import time, so that help(), Sphinx, and static
    tooling keep working and import stays cheap.
  26. As a maintainer, I want the queryables-reading logic to stay in the one
    protocol-level place that already owns it, so that this does not become a
    second implementation of the same parsing.
  27. As a maintainer, I want NGWMN — the other OGC adapter — to be able to adopt
    the same check without rewriting it, so that the mechanism generalises the
    way the OGC machinery already does.
  28. As a maintainer, I want the deprecated NWIS getters excluded, so that a
    frozen legacy surface is not held to a standard its service cannot answer.
  29. As a maintainer, I want the snapshot to stay human-readable and diffable in
    review, so that an upstream wording change is legible in a pull request.
  30. As a maintainer, I want the snapshot's format change to keep the existing
    name-drift assertion working, so that this extends the current monitor
    instead of replacing it.
  31. As a maintainer, I want a clear record of which check blocks and which
    advises, so that a red mark is unambiguous about whether it stops a merge.
  32. As a maintainer, I want the contributor guide to explain how to read a
    drift failure and how to refresh, so that the procedure is not folk
    knowledge.
  33. As a release manager, I want documentation drift surfaced before a release
    rather than after, so that a published version does not ship prose the
    service has already contradicted.
  34. As a downstream library author, I want the descriptions in this package to
    be reliable enough to quote, so that I can build my own docs on top of them.

Implementation Decisions

Docstrings stay hand-written literals. The service is the authority a check
consults, not a source docstrings are generated from. Runtime __doc__
rewriting, import-time fetching, and decorator-based documentation are all
rejected: they break help() reliability, defeat static tooling, add import
cost, and would make the docstring stop being the thing in the source. This is
the substantive difference from the R package's approach and it is deliberate.

No new module in the shipped package. The comparison is a check, not a
capability, so it lives in test support. This matters concretely: the dependency
contracts declare exhaustive = True over the package's top-level modules, so a
new one fails the layer contract until it is placed in the stack deliberately.
There is no reason to spend that placement on a test helper. ADR 0003 and ADR
0006 are unaffected.

Queryables parsing is not reimplemented. dataretrieval.ogc.schema already
reads an OGC queryables document into a frame of queryable, type, title,
description, and already takes the API base URL as an argument so any OGC
adapter can use it. The check consumes that. If it needs a shape the frame does
not offer, extend the existing function rather than parsing the document again.

The snapshot grows from names to definitions. It currently maps each
collection to a sorted list of queryable names. It becomes a mapping from
collection to queryable to the type, title, and description the service
publishes. Sorted keys and indented JSON, so a wording change reads as a small
diff. The existing name-drift assertion continues to work by reading the keys.

The set of getters under check is derived, not listed. The collection-family
inventory already exists in one place — the dependency contract that declares
the families independent, which the architecture suite already reads to derive
the facade's expected export union. The check resolves getters through the Water
Data facade and derives the argument set by introspecting signatures, so a new
getter or family is covered on the day it is added rather than when someone
extends a list.

Getter-to-collection mapping is explicit. Introspection can find a getter's
arguments but not which collection it targets. That mapping is declared once, in
test support, next to the check that uses it. A getter with no declared
collection fails the check rather than being skipped, so the mapping cannot go
stale silently.

Package-owned arguments are declared once and exempted. convert_type,
max_rows, limit, properties, skip_geometry, filter, filter_lang,
bbox, and ssl_check are package concepts or OGC-standard request controls,
not collection queryables. They are listed in one place as exempt from the
service comparison. Their cross-getter consistency is a separate concern and is
out of scope here.

Agreement means containment after normalisation, not equality. The
documented description must contain the service's description once both are
normalised — whitespace runs collapsed, wrapping and trailing newlines ignored,
case preserved. This lets a docstring carry the service's text plus our own
usage note or Python-specific caveat, which several already do and which is the
reason a strict-equality rule would be rejected on contact. A description that
has dropped or contradicted the service's text fails.

Three findings, three severities. A documented argument whose description no
longer contains the service's text is a failure. A service queryable with no
corresponding typed argument is a report, not a failure — promoting a
queryable to a typed argument is a design decision, not a correction. A typed
argument that is neither exempt nor a known queryable is a failure, because
it means the getter-to-collection mapping or the exempt list is wrong.

The offline gate and the live check are separate, mirroring existing
practice.
The offline comparison joins the merge gates that already run on
every push. The live comparison — snapshot against service — joins the weekly
advisory sweep, continue-on-error, reporting to the run summary. This is the
same gate/advisory split the repository already draws between its blocking
workflow and its weekly structural one, and the same offline/live split the
suite already draws with its live marker.

Refresh is one command. The snapshot regeneration procedure currently lives
as a heredoc in a module docstring. It is promoted to a real developer entry
point in test support that fetches every collection's queryables, rewrites the
snapshot, and prints a report: which queryables appeared, disappeared, and
changed description, and for each changed one, which getters document it. The
command is what both a maintainer and the weekly workflow invoke, so the report
a human reads and the report CI posts are the same code.

Scope of services. Water Data now. NGWMN is the same OGC machinery and
should be able to adopt the check by supplying its base URL and its own
snapshot, so nothing in the design may hard-code the Water Data base URL below
the mapping layer. WQP, NLDI, NWDC, and StreamStats publish no queryables
document and are out of scope. NWIS is deprecated and quarantined by contract;
excluded.

Testing Decisions

What a good test looks like here. The thing under test is a published
artifact — the text a caller reads — so the assertion is on external behaviour:
given the getters the package exports and the descriptions the service
publishes, do the two agree? The check reaches for getters through the public
facade and for descriptions through the existing schema reader, never through a
private helper, and never asserts how a docstring came to say what it says.
Failures name the collection, the queryable, and the getter, and print both
texts, so the log is the diagnosis.

Seams. One, and it already exists. The Water Data queryables test module
already owns the question "does our view of the queryables match the service?"
— it holds the committed snapshot, the live monitor, and the regeneration
procedure. The docstring comparison is the same question asked of a second field,
so it belongs there rather than in a new module. No new production seam is
introduced; nothing in dataretrieval/ is added or moved.

The public-API contract module was considered and rejected as the home. It
deliberately asserts properties of signatures rather than rendered text, and
it is offline and fixture-free by design; docstring prose is rendered text, and
this check is fixture-driven. Putting it there would blur a boundary the module's
own docstring draws explicitly.

Modules exercised. The Water Data facade and its collection-family getters
(time series, metadata, measurements, samples, ratings, stats) through their
public names; dataretrieval.ogc.schema as the queryables reader. Nothing else.

Prior art in this repository, to follow rather than reinvent.

  • The existing snapshot-versus-live monitor in the queryables test module: the
    committed-snapshot pattern, the regeneration procedure, and the convention of
    a docstring that explains what a failure means and how to respond.
  • The architecture suite: deriving the set of things under check from a single
    declared inventory instead of a hand-maintained list, so new code is covered
    by default.
  • The public-API contract module: asserting durable properties rather than
    character-exact snapshots, and writing out only the lists whose editing should
    be a deliberate act.
  • The live marker and its scheduled workflow: how a network-dependent check is
    kept out of the default run and given its own schedule.
  • The weekly advisory workflow: continue-on-error, a written run summary, and
    an explicit note that findings are leads rather than verdicts.
  • The suite's offline default: HTTP mocked with httpx_mock against captured
    fixtures, so the new offline gate makes no request at all.

Out of Scope

  • Generating, interpolating, or rewriting docstrings — at import time, at build
    time, or in CI. The R package's doc-generation approach is understood and
    deliberately not adopted.
  • Auto-committing docstring edits. Refresh rewrites the snapshot and reports;
    a human writes the prose.
  • Deduplicating the ~37 KB of verbatim-repeated parameter prose. This spec makes
    the duplication checked; consolidating it is a separate change with a real
    cost to help() readability and deserves its own decision.
  • Cross-getter consistency of package-owned arguments (max_rows, limit,
    convert_type). They are exempted here, not unified.
  • WQP, NLDI, NWDC, StreamStats, and deprecated NWIS.
  • Promoting newly discovered queryables to typed arguments. Reported, not done.
  • Any change to public signatures, return shapes, or the (DataFrame, metadata)
    contract.
  • Adding a documentation dependency such as numpydoc to parse docstrings; a
    section split adequate for numpydoc parameter blocks is a few lines and does
    not justify a dependency.
  • Coverage thresholds, coverage publication, and the release-stability policy —
    separate findings from the same review, separate tickets.

Further Notes

This came out of a criteria-by-criteria comparison of this package against the
R dataRetrieval package. That review put this package ahead on nearly every
internal measure — strict typing, enforced dependency contracts, complexity
ratchets, an offline mocked suite with a live monitor, a multi-OS matrix — and
behind on user-facing documentation, where the R package has both greater breadth
and the schema-interpolation guarantee. This ticket takes the guarantee, which is
the transferable part; the breadth gap is a writing task and is tracked
separately.

Constraints an implementer should know before starting:

  • The dependency contracts declare exhaustive = True over top-level package
    modules. Adding one to dataretrieval/ fails the contract until it is
    placed in the layer stack deliberately. This is a strong reason to keep the
    work in test support, and a thing to check early if that decision is revisited.
  • ADR 0003 (dependency direction) and ADR 0007 (adapter facades) govern this
    area. Nothing here should need to contradict either; if it does, raise the
    conflict rather than routing around it.
  • The complexity ratchets apply to the package only, not to tests, so a branchy
    comparison helper in test support is not a gate problem.
  • time_series.py at ~80% docstring by bytes is the module most affected. It is
    the natural first target and the best early signal of whether the containment
    rule is calibrated correctly.
  • The glossary term is queryable, and collection rather than service
    for daily, continuous, monitoring-locations. service means the external
    system. Getter output stays (DataFrame, metadata).

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationQuestion, request, or suggestion for additional documentationready-for-agentFully specified and ready for an AFK agent

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions