Skip to content

Extend ruff rule selection for the analytics package #4942

Description

@hunterckx

(Text courtesy of Claude)

Follow-up from #4934 / #4936, which added ruff to the analytics package with a deliberately minimal rule selection.

Problem

analytics/pyproject.toml selects ["E4", "E7", "E9", "F", "I", "W", "B"] — enough to catch the star-import re-export leak that motivated #4934, but well short of what we already run on Python in clevercanary/hca-validation-tools (ruff.toml):

select = ["E", "F", "I", "W", "B", "C4", "PIE", "PTH", "RET", "RUF", "SIM", "UP"]

The gap is C4, PIE, PTH, RET, RUF, SIM, UP. Two Clever Canary Python codebases linting to two different standards means review habits don't transfer, and the analytics package silently accumulates patterns that wouldn't survive review in the other repo.

Suggested fix

Extend the selection in analytics/pyproject.toml to:

select = ["E4", "E7", "E9", "F", "I", "W", "B", "C4", "PIE", "PTH", "RET", "RUF", "SIM", "UP"]

One intentional deviation from hca-validation-tools: keep E limited to the existing E4/E7/E9 subset instead of taking all of E. The non-preview rules that full E would add over that subset are exactly E501 (line-too-long) and E101 (mixed-spaces-and-tabs) — both formatting concerns, which are ruff format's job. Keeping them out means the linter never duplicates or fights the formatter.

Scope is the select list only. hca-validation-tools' other lint settings (per-file-ignores, isort.known-first-party, line-length, target-version) are out of scope here; add a per-file-ignores entry only if a real violation makes it necessary, and leave formatting config alone.

Commit structure

Enable one rule category (or group of no-op categories) per commit, fixing its violations in the same commit, so each category reviews independently and a regression can be traced to one rule. Counts below are from ruff check --select <cat> --statistics against main at e738381, using the pinned ruff==0.16.3:

# Category Violations Notes
1 C4, PIE, UP 0 Config-only commit — nothing to fix, no code touched.
2 SIM 1 SIM108 in analytics/api.py — one if/else → ternary.
3 RUF 2 RUF005 (tuple concatenation, api.py), RUF022 (unsorted __all__).
4 RET 5 RET504 (unnecessary assign before return), 2× RET505 (superfluous else return).
5 PTH 33 os.path / os / globpathlib. All in static_site/export.py (26) and static_site/generator.py (7).

Notes on the PTH commit: it is the bulk of the work and is a real refactor, not a mechanical fix — only 3 of the 41 total violations have safe autofixes, and PTH118/PTH123 (os.path.join, open) account for 27 of the 33. Keep it last so the smaller categories are already merged-ready if it needs more review discussion, and keep it to the path-handling change — no unrelated cleanup in that commit.

Verification

  • npm run lint:python and npm run check-format:python pass clean, and the same steps pass in run-checks.yml CI.
  • Fresh-venv generate_static_site.py run including historic_data_path, per the chore: retire legacy analytics formats — tracking #4913 verification convention. PTH rewrites path construction and file I/O in export.py and generator.py, so the generated site output needs to be confirmed unchanged — not just importable.

Metadata

Metadata

Assignees

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