Skip to content

Migrate tooling to ruff, ty, and uv - #173

Merged
skearnes merged 7 commits into
mainfrom
modernize-tooling
May 12, 2026
Merged

Migrate tooling to ruff, ty, and uv#173
skearnes merged 7 commits into
mainfrom
modernize-tooling

Conversation

@skearnes

@skearnes skearnes commented May 10, 2026

Copy link
Copy Markdown
Member

Summary

Mirrors the tooling migration done in ord-schema (#780, #783):

  • Replace black/isort/pylint/pytype with ruff + ty
  • Manage dependencies with uv ([dependency-groups] dev)
  • Consolidate setup.py into pyproject.toml; delete setup.py and format.sh
  • Add .pre-commit-config.yaml (addlicense + ruff-check + ruff-format + ty)
  • Bump CI test matrix from 3.10–3.12 → 3.11–3.14 (matches ord-schema); bump numpy<2numpy>=1.26.4 for cp313/cp314 wheels; widen protobuf==4.22.3protobuf>=4.22,<6
  • New checks.yml (replaces cleanup.yml); tests.yml uses astral-sh/setup-uv@v5 + uv sync --frozen
  • Update Dockerfile to use uv sync --frozen --no-dev

Source changes

  • Strip 8 # pylint: disable=... and 1 # pytype: disable=... comments from non-editor files
  • Fix all 19 ty diagnostics surfaced by the new type checker:
    • Parameterize psycopg connection/cursor with dict[str, Any] rows (DictCursor alias)
    • Fix QueryResult.__eq__ Liskov violation
    • Annotate kwargs: dict[str, Any] in a test
    • Generalize _pbtxt to accept any protobuf Message
    • cast(Chem.Mol | None, ...) around mol_from_compound (preserves the subsequent if mol: null-check; return type widens with return_identifier=True)
    • Annotate yielding pytest fixture as Iterator[Reaction]
    • cast(Awaitable[bool], ...) around redis.asyncio client.ping() (stub returns Awaitable[bool] | bool)

Packaging

  • Move testing-postgresql from runtime dependencies to the dev group; lazy-import Postgresql in api/main.py inside the ORD_INTERFACE_TESTING=TRUE branch (the only place it's instantiated)

Editor scope

The editor (and its entrypoint ord_interface/interface.py) is slated for removal on a parallel branch. To avoid churning soon-to-be-deleted code and creating merge conflicts, those paths are excluded from ruff/ty here. They still ship and run as before.

Test plan

  • CI: Checks workflow passes (ruff check + format, ty, addlicense, clang-format-js)
  • CI: Tests workflow passes on 3.11, 3.12, 3.13, 3.14 (Linux + macOS)
  • CI: test_app workflow (Docker / Vue) passes on 3.11–3.14
  • Local: uv run pytest ord_interface/visualization passes (verified)
  • Local: uv run pytest ord_interface/api passes against a real postgres+rdkit cartridge
  • Local: docker build -f ord_interface/Dockerfile -t openreactiondatabase/ord-interface . succeeds

🤖 Generated with Claude Code

skearnes and others added 5 commits May 9, 2026 20:17
Mirrors the migration done in ord-schema (#780, #783): replaces black /
isort / pylint / pytype with ruff + ty, switches dependency management
to uv, consolidates setup.py into pyproject.toml, and adds pre-commit.

Tooling
- pyproject.toml: full [project] block + [dependency-groups] dev;
  [tool.uv] package = false; [tool.ruff] / [tool.ty.src] config matching
  ord-schema. Drop [tool.black]/[tool.isort]/[tool.pytype]. Drop setup.py
  and format.sh.
- Add .pre-commit-config.yaml (addlicense + ruff-check + ruff-format + ty).
- Generate uv.lock; bump numpy<2 -> numpy>=1.26.4 to enable cp313/cp314.

CI
- New checks.yml replaces cleanup.yml: ruff check + ruff format --check +
  ty + addlicense + clang-format.
- run_tests.yml uses astral-sh/setup-uv@v5 + uv sync --frozen; matrix
  bumped from 3.10-3.12 to 3.11-3.14 (matches ord-schema).

Source
- Strip 8 # pylint: disable and 1 # pytype: disable comments from
  non-editor files.
- Exclude ord_interface/editor/ and ord_interface/interface.py from
  ruff/ty (slated for removal in #N).
- Fix all 19 ty diagnostics:
  * Type psycopg connection/cursor as dict[str, Any] rows (introduces
    DictCursor alias in queries.py).
  * Fix QueryResult.__eq__ Liskov violation (other: object + isinstance).
  * Annotate kwargs: dict[str, Any] in queries_test.test_similarity_query.
  * Generalize _pbtxt to accept any protobuf Message (callers pass both
    Reaction and ProductCompound).
  * cast(Chem.Mol, ...) around mol_from_compound (return type widens to
    a tuple when return_identifier=True).
  * Annotate yielding fixture as Iterator[Reaction].
  * One # ty: ignore[invalid-await] on redis.asyncio client.ping()
    (stub returns Awaitable[bool] | bool).

Dockerfile
- Replace setup.py egg_info + pip install dance with
  uv sync --frozen --no-dev; drop the final pip install . (package=false).
Rename run_tests.yml to tests.yml, restrict push trigger to main, and
drop the unused CACHE_TARGET env var.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Bump protobuf cap from <5 to <6 so resolution picks 5.29.6, which
  ships cp38-abi3 wheels and runs on Python 3.14. The 4.x series has no
  3.14 wheels, so import fails with "Metaclasses with custom tp_new are
  not supported." Matches ord-schema's constraint.
- test_app: invoke run_tests.sh via "uv run" so the venv's python and
  pytest are on PATH. The script's bare "python client/build_database.py"
  was hitting the system interpreter, which doesn't have ord_interface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The pyproject sets [tool.uv] package = false and has no [build-system],
so uv sync only installs dependencies, not the ord_interface package
itself. Pytest's rootdir handling masked this in test_ord_interface,
but the test_app job runs build_test_database.sh which calls
"python client/build_database.py" directly and hits ModuleNotFoundError.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
protobuf 5.x's MessageToBytes encodes the string form as ASCII by
default and raises UnicodeEncodeError when the message contains
non-ASCII content. as_utf8=True keeps non-ASCII bytes through to the
output, matching the prior 4.x behavior in practice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@skearnes
skearnes marked this pull request as ready for review May 10, 2026 00:52
@skearnes
skearnes requested a review from bdeadman May 10, 2026 00:53
skearnes and others added 2 commits May 9, 2026 20:58
- Add [build-system] (setuptools+wheel) and [tool.setuptools.packages.find]
  scoped to ord_interface*; drop [tool.uv] package = false. uv sync now
  installs ord-interface as an editable wheel into the venv, so
  "import ord_interface" works from anywhere instead of requiring the
  repo root on sys.path.
- tests.yml: drop the test_app PYTHONPATH workaround that compensated
  for the project not being installed.
- tests.yml: add fail_ci_if_error to codecov-action so fork PRs and
  dependabot/* branches don't fail the job on tokenless uploads.
  Mirrors ord-schema's run_tests.yml.
- Dockerfile: split uv sync into a deps-only pass before COPYing
  sources (--no-install-project) and a final sync after to install the
  project itself, preserving layer caching for deps.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Apply review feedback:

- filters.py: cast(Chem.Mol | None, ...) instead of cast(Chem.Mol, ...)
  to preserve the meaning of the subsequent `if mol:` null-check.
- search.py: replace `# ty: ignore[invalid-await]` on
  `redis.asyncio.Redis.ping()` with `cast(Awaitable[bool], ...)`,
  scoping the workaround to the actual problem (stub union return type).
- pyproject.toml/uv.lock: move `testing-postgresql` from runtime
  dependencies to the `dev` group; lazy-import `Postgresql` in
  `api/main.py` inside the `ORD_INTERFACE_TESTING=TRUE` branch since
  that's the only path that instantiates it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@skearnes
skearnes merged commit 048ad79 into main May 12, 2026
15 checks passed
@skearnes
skearnes deleted the modernize-tooling branch May 12, 2026 22:46
skearnes added a commit that referenced this pull request May 13, 2026
* Remove stale config and accidental commits

- .pylintrc, .style.yapf: legacy linter/formatter configs replaced by
  ruff in #173.
- copilot/: AWS Copilot CLI manifest, replaced by Pulumi/ECS in the
  ord-infrastructure repository.
- dump.rdb: accidentally committed Redis snapshot.
- README.md, CONTRIBUTING.md: setup.py / pip install instructions
  rewritten in terms of uv now that #173 has landed.
- .gitignore: add .pytest_cache/, .ruff_cache/, .venv/, dump.rdb so
  these stop showing up in future working trees.

Editor-tree `# pylint:` / `# pytype:` comments are intentionally left
alone; the editor is excluded from ruff/ty and is being removed in #172.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Refresh README

- Drop the placeholder "How to Deploy" section.
- Fix the project layout: api/ is the FastAPI server (not /client),
  and the description was missing it entirely. Mark editor/ as
  legacy (being removed in #172).
- Tighten setup instructions and folder descriptions; trim filler.
- Flag the host-port 5432 collision for `docker compose up`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Apply review feedback on cleanup PR

- CONTRIBUTING.md: sweep remaining `ord-schema` references to
  `ord-interface` (title, issue-tracker URLs, fork instructions). The
  goals/non-goals readthedocs link is left as-is and rephrased to
  point at "the broader Open Reaction Database" so it's clear the
  reference is intentional.
- README.md: add a brief Deployment section pointing at the
  ord-infrastructure repo (Pulumi/ECS) now that the old "How to
  Deploy" placeholder is gone.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* .gitignore: add trailing slashes to directory-only patterns

Make the pre-existing directory patterns consistent with the new ones
added in this branch (`.pytest_cache/`, `.ruff_cache/`, `.venv/`) and
the already-slashed ones above (`.idea/`, `docs/_build/`, `.vscode/`).

Trailing slash restricts a pattern to match directories only, which
is what we actually want for `__pycache__`, `*.egg-info`,
`.ipynb_checkpoints`, `build`, `dist`, `ketcher`, `standalone`,
`node_modules`, and `coverage`. A future file accidentally named one
of these wouldn't be silently ignored.

`**/.pnp` is left as-is because Yarn 2+ uses `.pnp.cjs` files while
Yarn 1 used a `.pnp` directory; the un-slashed form matches both.

Verified `git check-ignore` still resolves the existing
`.venv/`, `ord_interface.egg-info/`, `ord_interface/__pycache__/`
directories through the new rules.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (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.

2 participants