chore: use uv and ruff for analytics reporting (#4934) - #4936
Conversation
Replace the bare setuptools setup.py with a pyproject.toml (setuptools backend, so the downstream git+#subdirectory install path is unchanged), and replace the hand-pinned requirements.txt with a uv workspace and uv.lock. Adds ruff config and a .python-version.
Auto-fixes for E714 (not-is-test), I001 (import sorting) and trailing whitespace, plus explicit strict=True on the two zip() calls flagged by B905 -- both zip sequences that are equal-length by construction.
Whitespace and wrapping only. Notably converts api.py from tab to space indentation -- it was the only tab-indented file in the tree.
Adds an analytics job to run-checks.yml that syncs the uv environment against the lockfile, runs ruff check and ruff format --check, and smoke-imports the package to prove the pyproject packaging installs.
Replaces the venv + pip + requirements.txt instructions with uv sync, and documents the ruff commands.
4e966c9 to
84d3588
Compare
| lambda m: f"{m[:4]}-{m[4:]}" if len(m) == 6 and "-" not in m else m | ||
| ) | ||
| counts_by_month = dict(zip(grouped[month_col], grouped[count_col].astype(int))) | ||
| counts_by_month = grouped.set_index(month_col)[count_col].astype(int).to_dict() |
There was a problem hiding this comment.
One change here that's not just reformatting: there was a linting issue with the zip so this is changed to keep the two columns together in Pandas and then convert to a dict
| return dict( | ||
| zip([dimension["id"] for dimension in dimensions], [dimension["alias"] for dimension in dimensions]) | ||
| ) | ||
| return {field["id"]: field["alias"] for field in fields} |
There was a problem hiding this comment.
Another zip issue -- here converted to a notably simpler dict comprehension
There was a problem hiding this comment.
Pull request overview
This PR modernizes the analytics/** Python tooling by migrating dependency management to uv, consolidating packaging metadata into pyproject.toml, introducing Ruff linting/formatting, and wiring Python checks into CI to prevent regressions like the prior star-import leak.
Changes:
- Replace
venv+requirements.txtworkflow withuv(uv.lock,.python-version) and update setup docs. - Add Ruff lint/format configuration and new npm scripts for running Python lint/format.
- Add a new
analyticsjob torun-checks.ymlto run Ruff and validate imports under a locked environment.
Reviewed changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds npm scripts to run Ruff via uv from the repo root. |
| analytics/requirements.txt | Removes legacy pip requirements file in favor of uv.lock. |
| analytics/readme.md | Updates local setup + lint/format instructions for uv + Ruff. |
| analytics/pyproject.toml | Defines uv workspace, dependency groups (incl. Ruff), and Ruff lint selection. |
| analytics/uv.lock | Adds the locked dependency resolution for the analytics workspace. |
| analytics/.python-version | Pins Python version used by uv. |
| analytics/lungmap/generate_static_site.py | Ruff-driven import and formatting updates. |
| analytics/lungmap/constants.py | Normalizes string quoting/formatting. |
| analytics/hca-explorer/generate_static_site.py | Ruff-driven import and formatting updates. |
| analytics/hca-explorer/constants.py | Reformats nested dict constant for readability. |
| analytics/anvil-explorer/generate_static_site.py | Ruff-driven import and formatting updates. |
| analytics/anvil-explorer/constants.py | Normalizes string quoting/formatting. |
| analytics/anvil-catalog/generate_static_site.py | Ruff-driven import formatting updates. |
| analytics/analytics_package/setup.py | Removes legacy setup.py packaging entrypoint. |
| analytics/analytics_package/pyproject.toml | Introduces setuptools-based packaging metadata via pyproject.toml. |
| analytics/analytics_package/analytics/static_site/generator.py | Import ordering/formatting changes. |
| analytics/analytics_package/analytics/static_site/fetch.py | Formatting plus a small dict construction change for monthly counts. |
| analytics/analytics_package/analytics/static_site/export.py | Formatting changes and minor readability improvements. |
| analytics/analytics_package/analytics/static_site/charts.py | Formats nested chart config structures for readability. |
| analytics/analytics_package/analytics/static_site/init.py | Reorders exports and formats __all__. |
| analytics/analytics_package/analytics/report_elements.py | Import ordering/formatting and readability refactors for chained operations. |
| analytics/analytics_package/analytics/entities.py | Minor formatting normalization for constants/enums. |
| analytics/analytics_package/analytics/api.py | Large formatting/indentation normalization; preserves existing behaviors. |
| analytics/analytics_package/analytics/_report_utils.py | Formatting + small refactor to renaming helper and readability improvements. |
| .gitignore | Switches ignored venv dir to analytics/.venv and ignores Ruff cache. |
| .github/workflows/run-checks.yml | Adds analytics CI job running Ruff and a minimal import check under uv. |
Suppressed comments (2)
analytics/analytics_package/analytics/api.py:404
- Spelling in comments: "nmes" → "names" and "Crete" → "Create".
# Collect column nmes
analytics/analytics_package/analytics/api.py:287
- Spelling in comment: "Crete" → "Create".
# Crete the dataframe
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
NoopDog
left a comment
NoopDog
left a comment
There was a problem hiding this comment.
Reviewed with Claude Code — the Python reformatting itself verified clean (AST-equivalent across all 16 changed files, pins preserved). A handful of tooling-hygiene suggestions, most inline below, plus one that doesn't anchor to the diff:
Pre-commit hook doesn't run the new Python checks — .husky/pre-commit gates prettier/eslint/tsc but not lint:python / check-format:python, so an analytics/*.py edit passes the hook and then fails in CI on ruff format --check. Unconditional wiring would break commits for developers without uv, but a guarded invocation (only when uv exists and analytics files are staged) would match how the other checks are integrated.
| *.egg-info/ | ||
| /analytics/**/site/ | ||
| /analytics/venv | ||
| /analytics/.venv |
There was a problem hiding this comment.
Replacing /analytics/venv with /analytics/.venv un-ignores venvs created under the old documented workflow (python -m venv ./venv inside analytics/). Anyone with such a venv will see it flood git status after pulling, and a stage-all could commit it. Suggest keeping both entries during the transition — it's free.
| "start": "npx serve out", | ||
| "lint": "eslint .", | ||
| "check-format": "prettier --check .", | ||
| "lint:python": "uv run --directory analytics ruff check .", |
There was a problem hiding this comment.
These plain uv run invocations auto-sync the full analytics environment (29 packages incl. pandas/numpy/google-api stack) just to run ruff. Adding --only-group dev, as the CI job already does, keeps a first-time npm run lint:python to the single ruff wheel instead of a multi-hundred-MB install.
| with: | ||
| enable-cache: true | ||
| - run: | | ||
| uv run --locked --only-group dev ruff check . |
There was a problem hiding this comment.
The ruff invocation is now spelled three different ways in three independently maintained places — here (uv run --locked --only-group dev), package.json (uv run --directory analytics, full sync), and the readme (plain uv run) — so local and CI don't run ruff under identical conditions, and a future flag/path edit to one copy but not the others produces local-pass/CI-fail drift. Consider one canonical entry point, e.g. CI calling the npm scripts, or the scripts adopting CI's flags.
| - run: | | ||
| uv run --locked --only-group dev ruff check . | ||
| uv run --locked --only-group dev ruff format --check . | ||
| uv sync --locked |
There was a problem hiding this comment.
This job materializes the uv environment twice: the two --only-group dev runs build a dev-only env for ruff, then uv sync --locked rebuilds the full env in the same job. Equivalent and simpler: one uv sync --locked first, then uv run --no-sync for all commands (ruff is in the default-installed dev group).
| dev = ["ruff==0.16.3"] | ||
|
|
||
| [tool.ruff.lint] | ||
| select = ["E4", "E7", "E9", "F", "I", "W", "B"] |
There was a problem hiding this comment.
This select re-spells ruff's default set (E4, E7, E9, F) before appending the real policy; extend-select = ["I", "W", "B"] expresses the same intent in one self-documenting line and tracks default-set updates when the pinned ruff is bumped. Explicit pinning is defensible too — nitpick either way.
There was a problem hiding this comment.
Ruff's default rules actually include an assortment of rules not included here, and do not include E4 or all of E7 and F. I don't really have a reason to prefer one choice or the other (or the combination), but a couple things to consider are that Ruff recommends an explicit select, and enabling all of the default rules would require a few more code changes. (On the other hand, using the default rules would be consistent with how we handle TypeScript linting)
Closes #4934
pyproject.tomlfiles for the package and for the broader analytics folderrun-checksworkflow (Ruff, lockfile consistency, importing the package entry point)