lint(ruff): select the checks no other gate covers, and pin the linter

[thodson-usgs]

Selects the ruff checks that catch something nothing else in the stack can see, fixes what they found, and pins the linter that grades them. Three live bugs; the suppression budget goes down, not up.

This started as a pylint adoption and turned into a measurement that pointed the other way — most of pylint's yield was ruff rules that simply were not selected. #399 covers what was left over after that.

The rules

Rule	Findings	Why it is not covered elsewhere
PLW1514	8 (1 in the package)	mypy does not model encodings; a locale-dependent write misbehaves only on the Windows leg, so a green Linux run proves nothing
ARG001	5	the check that notices when retiring a legacy getter (ADR 0005) strands a documented keyword
D100–D104	1	automodule ... :members: renders a docstring-less public symbol as a bare signature
BLE001	0	already half-adopted — see below
RUF100	0 (6 before this PR)	nothing was watching the suppressions. Six # noqa: BLE001 directives sat in the tree suppressing a rule nobody had selected
ASYNC	1	the suite awaits mocked transports, so a stalled event loop still passes
PD	4	inplace= is on pandas' way out; this is a pandas library
DTZ	1	a naive datetime in a library about time series
C4 LOG G T20 FA NPY	0	free ratchets at today's setting

BLE001 is the interesting one. Ruff reported all six pre-existing directives as Unused noqa directive (non-enabled: BLE001). Four were on handlers that re-raise and never needed one; those are deleted, and the two on progress.py's best-effort cosmetics now do something. Four directives removed, one added (a DTZ007 that is correct-by-design, see below).

What they found

• A silent data loss. get_ratings(file_path=...) wrote each rating with open(..., "w") and no encoding. On a non-UTF-8 locale an unrepresentable character raises UnicodeEncodeError → a ValueError → caught by _download_all's per-feature handler and downgraded to a SkippedRatingWarning, so the rating silently disappears from the returned dict. That function's own docstring promises the opposite. It had survived ruff, mypy --strict, 995 tests and a 98.9% coverage floor.
• A stalled event loop. The same line was a blocking open() inside async def _fetch_rating, which runs dozens-concurrent under a fan-out drive — so every rating write stalled every other in-flight download for its duration. Now _write_rating on a worker thread via anyio.to_thread.run_sync.
• A documented no-op. get_record's wide_format, datetime_index and state have read nothing since qwdata, gwlevels and water_use were retired, but the docstring still described the behavior they used to have — so get_record(sites=..., state="OH") reads as a state filter while doing nothing.
• A side effect that was already inconsistent. format_response set the index on the caller's frame in place while already returning a new object — and already left the caller's frame untouched on the peaks and GeoDataFrame paths, so the in-place branch was the odd one out.

The flow, not just the rules

The rule list is only half the gate. Four flow defects, none of which selecting rules addresses:

• The version was pinned in three places and honoured in none. The pre-commit rev, CI and the test extra all said 0.16.1; the machine that wrote this had 0.16.5 on $PATH and 0.15.12 in its venv, and ruff format --check disagreed by 28 files between them. The pin now lives in two places instead of three: a [lint] extra that CI installs the way the [metrics] job already installs its own, and the ruff-pre-commit rev. The test extra reuses dataretrieval[lint], exactly as it already reuses dataretrieval[type-check], so a bump is a two-line edit. (A required-version guard turned a mismatch into a hard exit 2; review pointed out it bought that at the cost of a fourth place to keep in step, and it is out.)
• preview = true enabled 52 rules to get one. PLW1514 is the only preview rule this config wants; without explicit-preview-rules every other preview rule under the selected prefixes is live too — 41 from E alone — and each ruff bump adds more. Not hypothetically: that count was 50 at 0.16.1 and is 52 at 0.16.5, four patch releases later. Those two would have arrived as CI failures nobody selected.
• Nothing watched the suppressions — RUF100, above.
• E501 was selected redundantly (already inside E) under a comment naming a line-length setting that did not exist. The value is now set explicitly, where it visibly governs both the checker and the formatter.

A note for authors, now in the config comment: RUF100's fix reads the rest of the line as the directive's description, so a co-located suppression for another tool is deleted with it. Write # noqa: X # pylint: disable=..., never # noqa: X, pylint: .... The four directives that carried a reason after a dash or in parens are respelled accordingly.

Two judgement calls worth your eye

1. get_record warns rather than raises. My first version raised NameError, matching how get_record already treats a defunct service. Review overturned it, correctly: a defunct service cannot return the data asked for, but get_record(service="dv", wide_format=False) returns exactly the right data — only the knob is dead. Raising would retire the parameters ~20 months before REMOVALS["nwis"] with no warned release in between. They now warn through _deprecation.warn_deprecated and are still ignored.
   They are not deleted from the signature: state would then fall through **kwargs to query_waterservices and onto the wire as a parameter NWIS does not define. (wide_format/datetime_index would not — to_str drops non-iterable scalars — so that argument holds for state alone.)

2. ADR 0005 says a deprecated surface should "warn once per user call" — and a call naming all three options now emits four DeprecationWarnings: one from @_deprecated plus one per option. They are not duplicates (each names a distinct subject and a distinct replacement, which the same ADR sentence asks for), but _warn_defunct_record_options raises through warn_deprecated directly and so does not pass through _deprecated's re-entrancy guard, which is the module's cadence mechanism. The count is now pinned by a test, as is the attribution the hand-counted stacklevel=4 encodes.

   Two ways to resolve, your call: clarify the ADR's cadence wording to scope the guarantee per deprecated surface (my preference — the code is doing what the Decision explicitly asks), or collapse the three advisories into one warning appended to the function-level message, which costs the per-argument subject/replacement pairing that makes each individually actionable.

One open question

The ratings write is still text mode (encoding="utf-8", newline="") writing response.text. httpx's TextDecoder uses errors="replace", so a body that is not valid UTF-8 round-trips through U+FFFD — which is normalization, not the byte-for-byte fidelity the test name claims. open(..., "wb") + response.content would be literally byte-for-byte and needs neither keyword. Practical impact today is nil (USGS RDB is ASCII, and httpx defaults to utf-8 when the asset carries no charset), and going binary would take the line out of PLW1514's scope, leaving that rule at zero findings in the package. Either is defensible; the write is now isolated in a three-line _write_rating, so it is a one-line change. Flagging rather than deciding.

Cost

Measured at ruff 0.16.1 (the pin at the time), interleaved A/B, n=25, whole tree: 25.9 ms → 25.6 ms, i.e. −0.3 ms, within noise. For scale, python -c pass is 121 ms on the same machine, and pre-commit's own per-hook orchestration is ~380 ms against the ~21 ms of ruff it wraps. The rules cost less than nothing measurable.

What was rejected, and why it matters

Around 3,500 findings across A, SLF, RET, DOC, FIX/TD, most of pydocstyle and 19 of 20 PL codes — none a defect. Several would actively damage the codebase: pydocstyle's section rules ship a safe autofix that rewrites eight flagship getters' docstrings into broken ones (they read the indented Examples: inside a parameter description as a section header); DOC502 wants thirteen public getters to delete the ChunkInterrupted entry that is the resume contract; PLR0913 fires 40 times on the OGC queryable signatures CONTEXT.md names as the public contract. PLC0414 must never be selected — mypy --strict implies no_implicit_reexport, so the 44 X as X aliases in configuration.py are required, and removing them produces 29 [attr-defined] errors.

Also rejected in the second pass: N818/N801 would rename exported NWIS_Metadata, RateLimited, ServiceUnavailable — a breaking change buying nothing. S is 1665 findings (1656 assert in tests; the two package hits are correct non-crypto retry jitter). ANN is 1722 and mypy --strict already covers the package. PYI019 had one finding but wants typing_extensions.Self — a new runtime dependency for one annotation on 3.10.

Caveats

• PLW1514 is the only selected rule ruff has not stabilized; it works via preview = true and is named explicitly under explicit-preview-rules, so a version bump has to re-check it. Re-checked at 0.16.5: still preview-only, still fires.
• DTZ007 in ogc/dates.py:_parse_datetime is correct-by-design — _DATETIME_FORMATS deliberately carries both the %z and the bare forms, and the docstring promises tz-awareness only for an input that carried an offset. It takes a directive naming the reason rather than a "fix".
• The tests/* ignore covers ARG001 and the five D1 codes — the largest admission here. pytest matches fixtures and hook arguments by name (renaming pytest_collection_modifyitems's config fails collection with a PluginValidationError), and test docstrings are not published API. Codes are named rather than written D1, so D105–D107 are not pre-exempted. The rationale lives in this description and the commit message rather than in the config, which review asked to keep to the constraint.
• Removing the mypy anyio override is unrelated to ruff but was found the same way: its rationale was a 3.9 target that a match statement would fail to parse, and python_version has been 3.10 since 1.2.0. mypy --strict passes without it, in the isolated pre-commit hook env as well.

Verification

All 11 CI checks pass, including the full 6-leg matrix (ubuntu + windows × 3.10/3.13/3.14). ruff check + ruff format --check clean at the pinned 0.16.5 · mypy --strict 60 files clean · 1146 tests pass · coverage 98.93% against the 98.9 floor · xenon, complexipy, import-linter (8/8 contracts) all pass.

Regression tests added for the fixes. The ratings test asserts the saved file equals the response body byte for byte using a character absent from cp1252, so it fails on the Windows leg without the fix and cannot fail on Linux or macOS.

Changes after review

• Ruff bumped 0.16.1 → 0.16.5, pin and ruff-pre-commit rev together. ruff check and ruff format --check are clean on the tree at that version, and both PLW1514 and ARG001 still fire.
• required-version removed and the test extra folded into dataretrieval[lint] — two places to keep in step, down from three on main and four on the first pass of this branch. The cost is that a contributor on a mismatched local ruff grades differently and quietly, as on main today, instead of exiting 2.
• The ruff config's comments are cut to the constraint (two or three lines each); the per-file-ignores note and the CI step's comment go entirely. The RUF100 author note about ruff's fix swallowing a co-located # pylint: disable= stays — that one is a trap you hit while editing the file, not history.
• The NEWS entry is two sentences: the two waterdata.get_ratings(file_path=...) bug fixes. The nwis.get_record option deprecations and the format_response change are no longer called out, nwis being deprecated itself.
• AGENTS.md now points at the [lint] extra rather than at the CI job for the pin.

Still open and unchanged: the ADR 0005 cadence question and the text-versus-binary ratings write below.

🤖 Generated with Claude Code

https://claude.ai/code/session_01T6MVcko4gh68LieGWxUnrR

[nodohs]

please address my suggestions

[nodohs]

lgtm

[thodson-usgs]

@ehinman , I updated the linter config and this PR fixes bugs and linting errors that resulted from that change. No new features.

[thodson-usgs]

Note for whoever merges this, from a separate project rather than a review point on the diff.

python-maintenance-loop carries two built-in guardrails — ruff-selection and ruff-config-file — that refuse any automated edit to a ruff configuration file or to a changed select/ignore line. Their stated reason is this PR series: the rule selection is a surface a human is actively arguing about, so a bot must not touch it.

That reason expires when #398 and #399 land. Worth revisiting the guardrails then rather than leaving a dated justification in place indefinitely — until they are retired, a lint lane there can only report drift and never act on it.

No change requested here.