Skip to content

Glossary violations across the package: full inventory for planning #406

Description

@thodson-usgs

Scan of the whole package against CONTEXT.md as #405 leaves it, so the total
cost is visible before anything is scheduled. Grouped by what a fix actually
costs, not by severity.

Ground rules used, both from ADR 0013:

  • Core terms (ours — chunk, page, source, fan-out, dialect, leaf)
    take one spelling everywhere, identifiers included. A second spelling is a defect.
  • Domain terms (monitoring location, collection) are fixed for prose only.
    An adapter keeps its service's spelling at its public surface, and that is not
    a violation. So nwis.get_record(service="dv") and wqp's Station stay.

What that leaves is below. Nothing here is a behaviour bug.


1. Internal, mechanical — no public surface

Safe to do in one PR each, or one PR total.

  • OgcDialect.cql2_services / date_only_services are keyed by collection
    (ogc/policy.py:45,48,65,66, read at ogc/requests.py:182,186). Already
    recorded under Known legacy names as the last two holdouts of the
    service→collection rename. Internal dataclass fields; renameable.
  • service_url names a collection's items URL — ogc/requests.py:173,227.
    Local variables.
  • Shared OGC machinery says site in prose. It serves both Water Data and
    NGWMN, so nothing there is about one service's spelling:
    ogc/shaping.py:93, ogc/planning.py:64,193,205, ogc/chunking.py:4,
    ogc/filters.py:60. Note planning.py uses site= / sites=[...] in
    illustrative URLs — Water Data's real parameter is
    monitoring_location_id, so those examples are stale as well as off-glossary.
  • Water Data adapter prose says site where its own parameter is
    monitoring_location_id: waterdata/measurements.py:196,204,212,255,341,352,559,
    waterdata/cql.py:118, waterdata/nearest.py:269, waterdata/ratings.py:138.
    Excluded deliberately: "site visit" (measurements.py:4,49) is a
    field-practice term of art, not the place — leave it.
  • batch used informally for a fan-out's set of chunks —
    exceptions.py:332,339, transport/fanout.py:520, waterdata/ratings.py:135,138.
    Lowest value on this list; listed for completeness.

2. The service= keyword on shared helpers — one decision, ~10 call sites

The largest single cluster, and the one worth deciding before the mechanical work.

Three shared entry points take a service argument:
ProgressReporter(service=) (progress.py:108), FanOut(service=)
(transport/fanout.py:255), and resolve_next_url(service=) (transport/links.py:44).

What callers actually pass:

call site passes actually a
ogc/chunking.py:264 args.get("collection") collection
ogc/engine.py:365 collection collection
ogc/engine.py:115 "OGC" protocol
nwdc.py:387 "nwdc" — beside adapter="nwdc" adapter
nwdc.py:434,438 "Water Use" service
waterdata/ratings.py:302,315,440 "ratings" collection
nwis.py:324 "peaks" collection

progress.py:117 concedes it in its own comment: "The service/collection being
retrieved (e.g. daily, peaks)"
. It is the progress line's leading label and
the error message's subject — it is whatever names the thing being retrieved,
which is why five different kinds of name arrive there.

CONTEXT.md currently blesses this and should not. Known legacy names says
service "still means the external system in transport and progress, where
it labels a progress line. That usage is correct." The call sites say otherwise —
nwdc.py:387 passes the same string as both service= and adapter=. That
sanction needs to go whichever way the rename lands.

Options, roughly: rename the parameter to what it is (label? subject?) and
let each caller pass its own accurate name; or keep service= and fix only the
callers that can honestly supply a service. Both are internal — none of these
three is public API. Needs a call before the Tier 1 work, since it touches the
same files.

3. Public surface — needs an ADR 0012 deprecation cycle

  • wqp.services_wqx3 / wqp.services_legacy (wqp.py:65,66) are public
    module-level lists holding WQP profiles (Result, Station,
    Activity) — collections by the glossary. Previously reviewed and
    deliberately left alone; on the list because it cannot be fixed without a
    deprecation.
  • wqp.wqp_url(service) / wqp.wqx3_url(service) — both in __all__
    (wqp.py:688,696), parameter is a profile/collection.
  • "<service default>" printed by show_configuration()
    (configuration.py:837). The glossary calls this an adapter default.
    User-visible output, so it is a compatibility question rather than a
    docstring edit. Raised in refactor: one word per concept in the configuration chain #400 and deliberately left there, since that PR's
    claim is that behaviour is unchanged.

4. Service vs adapter in the configuration modules

~11 sites where prose says service for what CONTEXT.md is emphatic is the
adapter ("The scope is the adapter, not the service and not the host"):
configuration.py:234,554,634,636,646,648, waterdata/configuration.py:6,34,47,60,
_configuration_core.py:28,225,393.

Not uniformly wrong — a base URL arguably is the service's even when the
setting is adapter-scoped, so this needs a judgement per site rather than a
sweep. #400 fixes a few in passing; the rest are open.

5. Settled — listed so they are not re-raised

  • waterdata.get_cql(service=) — deprecated, removal 2027-08-09.
  • waterdata.WATERDATA_SERVICES — permanent alias of WATERDATA_COLLECTIONS.
  • waterdata.get_samples(service=) and SERVICES — names a resource, not a
    service or a collection; kept by decision, reasoning in Known legacy names.
  • waterdata.get_codes(code_service=) — reproduces the Samples API's own word.
  • utils.query — one request, not a query; frozen public path.
  • dataretrieval.nwis service= throughout — ADR 0005 quarantine, frozen.
  • NGWMN "provider" — that service's own vocabulary.
  • ChunkInterrupted, ChunkedCall — permanent aliases.

Suggested order

Decide §2 first (it dictates edits in the same files as §1), then §1 as one or
two PRs, then §4 site by site. §3 is a separate deprecation-cycle decision with
no deadline pressure.

Depends on #405 for the rules this is scanned against.

Activity

  1. thodson-usgs commented on Sep 1, 2026

    @thodson-usgs
    CollaboratorAuthor

    Measured: two candidate pre-commit hooks

    Run against the repo the way the doubled-word hook (#404) was evaluated before
    adoption, and the way pylint and vulture were evaluated before rejection —
    every hit classified, negative controls included.

    Hook A — forbidden core-term identifiers

    \b(cql2_services|date_only_services)\b

    60 files scanned, 8 hits, 8 real, 0 false positives. Precision is 100% by
    construction: a literal identifier list cannot misfire. Negative control (seeded
    dialect.cql2_services = frozenset()) is caught.

    All 8 are the §1 item already in this issue — ogc/policy.py:45,48,65,66,
    ogc/requests.py:182,186, waterdata/utils.py:82,83.

    Deliberately excluded: services_wqx3 and services_legacy. They are public
    (§3) and can't be fixed without a deprecation cycle, so listing them now would
    leave the hook red indefinitely. They join the hook when their deprecation
    completes, not before.

    Hook B — site in shared machinery

    (?<!call )(?<!read )(?<!Call )\b[Ss]ites?\b(?! visit) over
    dataretrieval/transport/ and dataretrieval/ogc/ only.

    19 files scanned, 6 hits, 5 real, 1 false positive (83%).

    The false positive is ogc/filters.py:60, and it has exactly one cause: the
    phrase "at the call site" wrapped across a line break, so the line-scoped
    exclusion cannot see the call. Reflowing that one docstring line takes the
    hook to 0 false positives. (Line-scoping is deliberate — --multiline was
    measured at 19 FPs / 0 real for the doubled-word hook and rejected.)

    Negative control passes; the exclusions correctly ignore call site, read
    site
    , and site visit.

    Wider scope rejected. Extending Hook B to the rest of the shared machinery
    (configuration, credentials, progress, combining, exceptions,
    interruptions, utils, _validation) gives 6 hits, ~2 real — roughly 33%.
    The noise is a second wrapped "call site" (_configuration_core.py:498), a
    docstring example using sites as a Python variable, and exceptions.py:303,
    whose "No sites/data found…" is this package's own message for a public
    exception class (NoSitesError) and mirrors the service's sentinel. That last
    one is public surface, not a prose fix. Keep the hook at transport/ + ogc/.

    The sequencing constraint

    Both hooks are red on main today — that is the point of them, but it means
    neither can be added as a standalone PR. Each has to land in the same commit as
    the fix it enforces
    , as a ratchet:

    • Hook A ships with the cql2_services / date_only_services rename (§1).
    • Hook B ships with the six OGC prose fixes (§1), plus the one-line reflow of
      ogc/filters.py:60.

    What this does and does not buy

    Together the hooks cover the §1 items in ogc/ and nothing else. They do not
    touch §2 (the service= keyword — undecided), §3 (public surface), or §4
    (service vs adapter, which needs judgement per site). That is the intended
    limit: ADR 0013 says a core term can be checked mechanically and a domain term
    cannot, and these check only the core half in the one scope where no service's
    vocabulary can apply.

    Worth noting what the measurement says about scope generally: the same pattern
    is 100%/83% precise in shared machinery and ~33% one directory up. A glossary
    check is viable exactly where the glossary is unambiguous, which is a narrower
    place than it first appears.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions