Repository navigation
Glossary violations across the package: full inventory for planning #406
Description
Activity
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)\b60 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_wqx3andservices_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 —
sitein shared machinery(?<!call )(?<!read )(?<!Call )\b[Ss]ites?\b(?! visit)over
dataretrieval/transport/anddataretrieval/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 thecall. Reflowing that one docstring line takes the
hook to 0 false positives. (Line-scoping is deliberate —--multilinewas
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 usingsitesas a Python variable, andexceptions.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 attransport/+ogc/.The sequencing constraint
Both hooks are red on
maintoday — 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_servicesrename (§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 (theservice=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.- Hook A ships with the
Scan of the whole package against
CONTEXT.mdas #405 leaves it, so the totalcost is visible before anything is scheduled. Grouped by what a fix actually
costs, not by severity.
Ground rules used, both from ADR 0013:
chunk,page,source,fan-out,dialect,leaf)take one spelling everywhere, identifiers included. A second spelling is a defect.
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")andwqp'sStationstay.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_servicesare keyed by collection(
ogc/policy.py:45,48,65,66, read atogc/requests.py:182,186). Alreadyrecorded under Known legacy names as the last two holdouts of the
service→collectionrename. Internal dataclass fields; renameable.service_urlnames a collection's items URL —ogc/requests.py:173,227.Local variables.
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. Noteplanning.pyusessite=/sites=[...]inillustrative URLs — Water Data's real parameter is
monitoring_location_id, so those examples are stale as well as off-glossary.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 afield-practice term of art, not the place — leave it.
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 sitesThe largest single cluster, and the one worth deciding before the mechanical work.
Three shared entry points take a
serviceargument:ProgressReporter(service=)(progress.py:108),FanOut(service=)(
transport/fanout.py:255), andresolve_next_url(service=)(transport/links.py:44).What callers actually pass:
ogc/chunking.py:264args.get("collection")ogc/engine.py:365collectionogc/engine.py:115"OGC"nwdc.py:387"nwdc"— besideadapter="nwdc"nwdc.py:434,438"Water Use"waterdata/ratings.py:302,315,440"ratings"nwis.py:324"peaks"progress.py:117concedes it in its own comment: "The service/collection beingretrieved (e.g.
daily,peaks)". It is the progress line's leading label andthe error message's subject — it is whatever names the thing being retrieved,
which is why five different kinds of name arrive there.
CONTEXT.mdcurrently blesses this and should not. Known legacy names saysservice"still means the external system intransportandprogress, whereit labels a progress line. That usage is correct." The call sites say otherwise —
nwdc.py:387passes the same string as bothservice=andadapter=. Thatsanction needs to go whichever way the rename lands.
Options, roughly: rename the parameter to what it is (
label?subject?) andlet each caller pass its own accurate name; or keep
service=and fix only thecallers 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 publicmodule-level lists holding WQP profiles (
Result,Station,Activity) — collections by the glossary. Previously reviewed anddeliberately 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 byshow_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.mdis emphatic is theadapter ("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 ofWATERDATA_COLLECTIONS.waterdata.get_samples(service=)andSERVICES— names a resource, not aservice 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.nwisservice=throughout — ADR 0005 quarantine, frozen.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.