Remove the deprecated load_*/save_* aliases - #909
Merged
Conversation
The load_*/save_* rename (#818, #833) left forwarding shims that warn and delegate. They shipped in v0.6.30 and have been available through two minor releases; this removes them: * ord_schema.parquet_dataset (whole module, an alias for ord_schema.parquet) * parquet.write_dataset, read_dataset, read_metadata, read_footer, read_reaction * message_helpers.write_message, write_dataset * templating.read_spreadsheet * resolvers.name_resolve Nothing in ord-schema used them. The two guides that named write_message and name_resolve now name save_message and resolve_name. The module-level shim was the one worth removing soonest: its __getattr__ returns Any, so a type checker accepts any attribute reached through ord_schema.parquet_dataset, including attributes that do not exist. Callers elsewhere: ord-data's convert_to_parquet.py and README (open-reaction-database/ord-data#263) and ord-interface's editor server (read_spreadsheet, name_resolve). ord-interface pins ord-schema<0.9, so it is insulated until it widens that bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
validate_dataset.py and build_dataset.py accepted --input as an alias for --input_pattern. ord-data's validation.yml still passes --input, but it invokes these scripts from a pinned ORD_SCHEMA_TAG checkout, so it keeps working until that pin moves and can switch to --input_pattern then. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
skearnes
added a commit
to open-reaction-database/ord-interface
that referenced
this pull request
Jul 28, 2026
) Both are forwarding aliases left by the load_*/save_* rename, and open-reaction-database/ord-schema#909 removes them. The replacements -- templating.load_spreadsheet and resolvers.resolve_name -- have identical signatures and are present in the pinned ord-schema 0.8, so both call sites are a rename. The pin stays at >=0.8,<0.9; widening it waits for 0.9.0 to ship. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
skearnes
added a commit
to open-reaction-database/ord-data
that referenced
this pull request
Aug 2, 2026
* Separate the code license from the data license The repository holds two different kinds of thing under one LICENSE. The datasets under data/ are CC-BY-SA-4.0, which is what CITATION.cff and the Hugging Face dataset card declare and what a data consumer needs. The code under scripts/ and .github/ had no license of its own, which left it implicitly under the same CC license -- a poor fit, since Creative Commons recommends against applying CC licenses to software, and the project's other code repositories are Apache-2.0. LICENSE stays CC-BY-SA-4.0 so GitHub keeps advertising the data license, which is the right headline for a data repository. LICENSE-CODE adds Apache-2.0 for the code, following the convention Microsoft uses for its docs and data repositories, and addlicense stamps per-file headers so a script copied out of the repository carries its license with it. This changes no code: the diff is 333 insertions and no deletions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Scope the addlicense hook to code files The hook ran unfiltered, so which files got an Apache header depended on addlicense's internal list of recognized extensions rather than on any stated intent. That list happens to exclude .md and .cff today, which is why README.md and CITATION.cff are unstamped -- but those files are CC-BY-SA-4.0 metadata describing the datasets, and "the tool does not recognize the extension" is not a guarantee worth resting a licensing boundary on. An allowlist states the boundary directly: code gets the code license, and anything else has to be added deliberately. Reported by Greptile on #261. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Add Python tooling: uv, ruff, ty, and pytest 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> * Keep the mirror job's --no-dev environment `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> * Move ord-data's pipeline scripts in from ord-schema process_dataset.py, pb_to_parquet_dataset.py, and parse_uspto.py are pipeline and one-off ingestion code for this repository rather than schema use cases; they arrive here with their tests. The matching removal is open-reaction-database/ord-schema#908. Test imports become bare `import X`, matching the co-located convention upload_to_huggingface_test.py already uses, and pb_to_parquet_dataset_test.py's caplog logger name follows __name__ to its new value. process_dataset.py's docstring called it "a one-stop shop for preparing submissions"; it is the submission workflow's machinery, and now says so. A `pipeline` dependency group holds what the dataset-touching scripts need -- ord-schema and pygithub -- and `dev` includes it, so the mirror job's `uv sync --no-dev` still skips the rdkit/pandas chain. submission.yml drops its pinned ord-schema checkout and pip install for `uv sync --only-group pipeline`, which also takes ORD_SCHEMA_TAG out of that workflow: the moved script imports ord_schema.datasets, which does not exist in v0.6.3, so the submission path necessarily validates at the locked 0.8. Full-corpus validation in validation.yml stays on the pinned tag. process_submission gains `needs: check_file_types`, because it runs process_dataset.py from the pull request's own checkout -- a fork could otherwise edit that script while the job rejecting non-dataset files was still running. Ruff's ignore list picks up the rules the incoming code trips (G004, PLR2004, RUF001, S101, PTH207, TD003, FIX002), each already ignored in ord-schema, and the test per-file ignore widens from D103 to D. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop deprecated ord-schema API from scripts and the README convert_to_parquet.py reached ord_schema.parquet through the parquet_dataset alias module and called write_dataset; the README's conversion example used message_helpers.write_message. All three are deprecated shims that open-reaction-database/ord-schema#909 removes. The README's JSON example imported write_message without using it, so that import goes rather than changing name. ty could not have caught the alias: parquet_dataset is a real module whose __getattr__ returns Any, so every attribute through it -- including ones that do not exist -- type-checks clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin setup-uv to a commit SHA A version tag is mutable: the tj-actions/changed-files compromise repointed every tag at a commit that dumped runner secrets into build logs, and only SHA-pinned consumers were unaffected. This job holds HF_TOKEN, which has write access to the public Hugging Face mirror. The trailing comment records the version so the pin stays readable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin submission.yml's setup-uv to a commit SHA Matches the pins the tooling PR added to tests.yml and huggingface_mirror.yml. This job runs with a GITHUB_TOKEN that pushes to the pull request branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Narrow the move to process_dataset.py parse_uspto.py and pb_to_parquet_dataset.py stay in ord-schema; only the submission pipeline's process_dataset.py belongs in this repository, since that is the script the submission workflow runs from the pull request's own checkout. Drop the ruff ignores that only the departing scripts needed: TD003 and FIX002 (no TODO comments remain), RUF001 (the non-ASCII unit-replacement table) and the repository-wide S101, which returns to the test-only scope where assert is the point of the code. G004, PLR2004, and PTH207 stay because process_dataset.py and its test still trip them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
load_*/save_*rename (#818, #833) left forwarding shims that warn and delegate. They shipped in v0.6.30 and have now been available through two minor releases, so they go:ord_schema.parquet_dataset(whole module)ord_schema.parquetparquet.write_datasetparquet.save_datasetparquet.read_dataset/read_metadata/read_footer/read_reactionparquet.load_dataset/load_metadata/load_footer/load_reactionmessage_helpers.write_messagemessage_helpers.save_messagemessage_helpers.write_datasetord_schema.datasets.save_datasettemplating.read_spreadsheettemplating.load_spreadsheetresolvers.name_resolveresolvers.resolve_nameThe deprecated
--inputalias for--input_patterngoes too, from bothvalidate_dataset.pyandbuild_dataset.py. ord-data'svalidation.ymlstill passes--input, but it runs these scripts out of a pinnedORD_SCHEMA_TAGcheckout, so it keeps working until that pin moves and switches to--input_patternat the same time.Nothing inside ord-schema used any of them, and no test asserted the warnings. The two guides that named
write_messageandname_resolvenow namesave_messageandresolve_name.Why the module shim was the urgent one
ord_schema.parquet_datasetforwards through a module-level__getattr__returningAny. That means a type checker resolves the module fine — it exists — and then accepts every attribute reached through it, including ones that do not exist:So a caller on the alias got no type checking of the API surface at all, and would have kept type-checking clean right up until it failed at runtime. That is exactly what happened in ord-data, where
convert_to_parquet.pyhad been on the alias since the rename withtyreporting nothing.Callers elsewhere
convert_to_parquet.pyand the README conversion example, fixed in ord-data#263.ord_interface/editor/py/serve.pyusestemplating.read_spreadsheetandresolvers.name_resolve. Migrated in ord-interface#213, merged.write_messageandname_resolve_cachedare its own functions, and it pins<0.7.This is a breaking change for anyone still on the old names, so it wants the next minor version rather than a patch.
Verified locally:
ruff check,ruff format --check,ty check, all pre-commit hooks; 534 tests pass.🤖 Generated with Claude Code
Greptile Summary
Removes deprecated API and CLI compatibility aliases.
ord_schema.parquet_datasetforwarding module and deprecated load/write/read function aliases.--inputspelling from the dataset build and validation CLIs.save_messageandresolve_name.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
Context used (3)