DOI-USGS / DOI-USGS/dataretrieval-python

Glossary violations across the package: full inventory for planning

Open
#406 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
265
Forks
63
Avg merge
1d 20h
Merged PRs (30d)
19

Description

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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.