DOI-USGS / DOI-USGS/dataretrieval-python
Glossary violations across the package: full inventory for planning
- Lingua principale
- Python
- Stelle
- 265
- Fork
- 63
- Merge medio
- 1g 20h
- PR unite (30g)
- 19
Descrizione
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](https://github.com/DOI-USGS/dataretrieval-python/pull/405):
- **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.
- [ ] `""` 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 #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.
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Read CONTEXT.md and ADR 0013 first, then inspect the shared entry points and call sites listed in §2 to understand the naming decision required before edits. After that decision, review the §1 and §4 files and tests, if any are added or identified; done means the agreed glossary corrections are complete without changing the deliberately preserved public names or compatibility behavior.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- python
- Ambito
- backend-api-design
- Tipo di issue
- Refactoring
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Stato di attività
- Attiva
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 30/100