Skip to content

Add Python tooling: uv, ruff, ty, and pytest - #262

Draft
skearnes wants to merge 2 commits into
license-splitfrom
python-tooling
Draft

Add Python tooling: uv, ruff, ty, and pytest#262
skearnes wants to merge 2 commits into
license-splitfrom
python-tooling

Conversation

@skearnes

@skearnes skearnes commented Jul 26, 2026

Copy link
Copy Markdown
Member

Stacked on #261 — review that one first; this PR retargets to main automatically when it merges.

ord-data has no Python tooling: no pyproject, no lockfile, no linter, no tests. Meanwhile scripts/ is about to receive process_dataset.py — which rewrites contributor submissions — and the derived-views driver. This stands up the harness before that code lands, mirroring ord-schema so the two repositories share one toolchain.

Dependencies

pyproject.toml replaces scripts/requirements.txt as the single dependency source, with uv.lock pinning the environment and CI installing via --locked. ord-schema is a dev dependency rather than a runtime one, so the mirror job can uv sync --no-dev and skip rdkit and pandas. Holding that requires uv run --no-sync on the run step, which the second commit adds: uv run otherwise syncs with the default groups first and reinstalls precisely what --no-dev excluded.

Lint config

The ruff select list is copied from ord-schema verbatim, since a shared standard across the two repositories is the point. The ignore list deliberately is not: ord-schema's ~40 exemptions are each justified by code this repository does not have, so this lists only rules that actually fire here — formatter conflicts, CLI realities like print and shelling out to git, and the handful ord-schema had already settled on style grounds.

That paid off immediately. Six functions and a class had no docstrings, one manual append loop became list.extend, and ty caught matches: callable in convert_to_parquet.py — a function used where a type belongs, now Callable[[dataset_pb2.Dataset], bool]. ty also flagged composed_readme.encode() on a str | None; that one is a false positive because an early return guards it, but the invariant was implicit, so deriving "there is something to mirror" from the composed card itself makes it structural.

Tests sit beside the modules they cover (foo.py / foo_test.py), matching ord-schema. The ten tests cover parse_name_status and build_configs — including that a rename must split into delete-old plus upload-new, since the mirror has no rename operation and would otherwise orphan the source.

Two things worth a look

  • The download instructions change. pip install -r scripts/requirements.txt becomes pip install huggingface_hub, with a uv run alternative alongside. It is the one line here that a data consumer sees.
  • ruff formats Python code blocks inside Markdown, which I did not expect — so a README example is reformatted in this diff. Harmless, but it explains a hunk that otherwise looks unrelated.

What this unblocks

Moving process_dataset.py and its 29 KB test suite out of ord-schema, per the logbook entry, and then the derived-views driver.

🤖 Generated with Claude Code

Greptile Summary

Adds a shared Python development and CI toolchain for the repository.

  • Defines and locks runtime and development dependencies with pyproject.toml and uv.lock.
  • Adds Ruff, ty, pytest, and pre-commit configuration plus a test workflow.
  • Migrates the Hugging Face mirror workflow from pip requirements to uv.
  • Adds tests for mirror diff parsing and dataset-card configuration generation.
  • Applies typing, formatting, and documentation updates to the helper scripts.

Confidence Score: 3/5

The PR should not merge until both setup-uv references are pinned to a reviewed full commit SHA.

The mirror and test workflows still execute a third-party action through a mutable major-version tag, so upstream tag movement can change CI code without repository review.

Files Needing Attention: .github/workflows/huggingface_mirror.yml and .github/workflows/tests.yml

Important Files Changed

Filename Overview
pyproject.toml Defines the virtual Python project, dependency groups, and pytest, Ruff, and ty configuration.
uv.lock Locks the Python development and mirror-job dependency environments.
.github/workflows/huggingface_mirror.yml Migrates the mirror environment installation and execution path from pip to uv.
.github/workflows/tests.yml Adds CI checks for linting, formatting, static typing, and tests.
scripts/upload_to_huggingface.py Adds annotations and documentation, refactors formatting, and preserves the existing mirror-planning behavior.
scripts/upload_to_huggingface_test.py Adds coverage for name-status parsing and Hugging Face dataset configuration generation.

Reviews (5): Last reviewed commit: "Keep the mirror job's --no-dev environme..." | Re-trigger Greptile

echo "Diff range: ${BASE}..${HEAD}"

- uses: actions/setup-python@v5
- uses: astral-sh/setup-uv@v7

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 security Mutable setup-uv action reference

The new mirror and test workflows execute astral-sh/setup-uv@v7 through a mutable major-version tag, allowing upstream tag movement to change the code executed by CI without review. Pin both references to a reviewed full commit SHA to make this supply-chain boundary immutable.

How this was verified: Both changed workflows invoke the mutable astral-sh/setup-uv@v7 reference directly.

@skearnes
skearnes marked this pull request as draft July 26, 2026 04:07
ord-data had no Python tooling -- no pyproject, no lockfile, no linter, no
tests -- while scripts/ is about to receive process_dataset.py, which rewrites
contributor submissions, and the derived-views driver. This sets up the harness
before that code lands, mirroring ord-schema so the two repositories share one
toolchain.

pyproject.toml replaces scripts/requirements.txt as the single dependency
source, with uv.lock pinning the environment and CI installing via --locked.
ord-schema is a dev dependency rather than a runtime one so the mirror job,
which only moves bytes, can `uv sync --no-dev` without pulling rdkit and pandas.

The ruff select list is copied from ord-schema verbatim, since a shared standard
across the two repositories is the point. The ignore list is not: ord-schema's
~40 exemptions are each justified by code this repository does not have, so this
lists only rules that actually fire here.

Running the toolchain surfaced real fixes rather than noise. Six functions and a
class had no docstrings; one manual append loop became list.extend; and ty
caught `matches: callable` in convert_to_parquet, a function used where a type
belongs. ty also flagged composed_readme.encode() on a `str | None`, which is a
false positive because an early return guards it -- but the invariant was
implicit, so deriving "there is something to mirror" from the composed card
itself makes it structural.

Tests sit beside the modules they cover (foo.py / foo_test.py), matching
ord-schema. pyproject.toml is added to the addlicense allowlist, since it
carries a header like the rest of the code.

Note that ruff formats Python code blocks inside Markdown, so a README example
is reformatted here.

The download instructions change: `pip install -r scripts/requirements.txt`
becomes `pip install huggingface_hub`, with a uv alternative alongside.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`uv run` syncs the environment before running, using the default groups, so the
bare `uv run python scripts/upload_to_huggingface.py` reinstalled the dev group
that `uv sync --no-dev` had just excluded -- including ord-schema and its
rdkit/pandas chain. Pass --no-sync so the run uses the environment the sync step
built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant