Delete unused reactions badge script#819
Merged
Merged
Conversation
badges/reactions.py generated a reaction-count shields.io badge for ord-data, which no longer uses it (the consuming workflow was just removed from ord-data). It has no callers, tests, or CI references in this repo, so remove it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #819 +/- ##
=======================================
Coverage 92.72% 92.72%
=======================================
Files 25 25
Lines 2665 2665
Branches 565 565
=======================================
Hits 2471 2471
Misses 120 120
Partials 74 74 🚀 New features to boost your workflow:
|
bdeadman
approved these changes
Jun 16, 2026
skearnes
enabled auto-merge (squash)
June 19, 2026 01:51
skearnes
added a commit
that referenced
this pull request
Jun 21, 2026
…et -> parquet (#818) * Add read_dataset with in-memory Parquet support Add a `read_dataset` dispatcher to message_helpers that mirrors `write_dataset`: `.parquet` files are read fully into memory via `parquet_dataset.read_dataset`, and all other suffixes route through `load_message`. This lets callers migrate datasets from `.pb.gz` to `.parquet` by changing only the filename — the returned fully-populated `Dataset` proto keeps downstream code unchanged. The Parquet path emits a UserWarning recommending the streaming `parquet_dataset.DatasetView` loader (or `iter_reactions`/`read_reaction`) for large datasets, since reading everything into memory defeats the format's streaming benefits. Kept separate from `load_message` because that function is generic over `message_type`, while Parquet only ever holds a `Dataset`; folding it in would break the generic return-type contract and add a Dataset-only special case. This also mirrors the existing `write_dataset`/`write_message` split. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Specify UserWarning category explicitly Address Greptile review: make the warning category self-documenting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: use read_dataset in Granda/Perera ML example Switch the example notebook's Dataset load from load_message(pb, dataset_pb2.Dataset) to the new read_dataset(pb) helper, and drop the now-unused dataset_pb2 import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Use read_dataset for plain Dataset loads in scripts Switch full-Dataset load_message(..., dataset_pb2.Dataset) call sites to the read_dataset helper in check_pb, pb_to_parquet_dataset, the reactions badge, and the ORM test fixture; drop now-unused dataset_pb2 imports. Left the streaming-aware loaders (process_dataset, validate_dataset, add_datasets) on their explicit non-parquet load_message branch: they already handle .parquet via DatasetView/iter_reactions/add_parquet_dataset, which is exactly the streaming path read_dataset recommends. build_dataset loads Reaction (not Dataset) and is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove VALIDATION_PLAN.md from branch This local working note was accidentally swept into the branch by a git add -A; untrack it so it stays out of the PR (file remains on disk). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Drop badges/reactions.py edit; file removed in #819 This file is deleted in #819, so the read_dataset conversion here is moot and would cause a modify/delete conflict. Restore it to the main version so #819 deletes cleanly regardless of merge order. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Rename read_dataset to load_dataset for consistency with load_message message_helpers already uses `load` as its read verb (load_message); the new dataset reader is its sibling, so load_dataset reads more consistently than read_dataset (which would be the only read_* symbol in the module). The lower-level parquet_dataset.read_dataset that it dispatches to is unchanged. Renames the function and all call sites (check_pb, pb_to_parquet_dataset, orm/conftest, the example notebook) plus the tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Use load_message in pb_to_parquet so .parquet inputs are rejected The converter turns serialized message-format Datasets (.pb/.pbtxt/.json/ etc.) into Parquet; a .parquet input is nonsensical. Switching from load_dataset (which would load it into memory and emit a misleading 'prefer the streaming DatasetView' warning) to load_message makes a .parquet input fail cleanly with an unsupported-format error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add ord_schema.datasets for whole-dataset I/O (load_dataset/save_dataset) Dataset-level file I/O previously lived in message_helpers, where the name collided confusingly with parquet_dataset.load_dataset/save_dataset and misrepresented these as message helpers. Move them to a dedicated ord_schema/datasets.py: - datasets.load_dataset / datasets.save_dataset are now canonical (format-dispatching: .parquet -> parquet_dataset, else message I/O). - load_dataset is brand new (this PR), so it just lands in datasets. - message_helpers.save_dataset was new in 0.6.29 with no real users, so it is dropped (no shim); the legacy message_helpers.write_dataset alias stays, repointed to datasets.save_dataset. - Fix stale calls: load_dataset now uses parquet_dataset.load_dataset / load_reaction (the canonical names), not the deprecated read_* aliases. Call sites (check_pb, process_dataset, ORM conftest, tests) updated; dataset I/O tests moved to datasets_test.py. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Rename ord_schema.parquet_dataset -> ord_schema.parquet With the new ord_schema.datasets module owning the high-level, format-dispatching dataset I/O, the lower-level Parquet backend reads cleaner as ord_schema.parquet (and datasets + parquet_dataset was an awkward overlap). Renames the module and its test, updates all internal imports, and leaves a back-compat ord_schema/parquet_dataset.py shim that forwards attribute access to the new module with a DeprecationWarning (removed in a future minor release). The pb_to_parquet_dataset script is unaffected (different name). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix Granda notebook: load_dataset moved to ord_schema.datasets Greptile: the notebook still called message_helpers.load_dataset, which now lives in ord_schema.datasets (would AttributeError on run). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (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.
Summary
Removes
badges/reactions.py, the script that generated a reaction-count shields.io badge for ord-data. ord-data no longer uses it — the generatedbadges/reactions.svgwas removed in open-reaction-database/ord-data#244.The script has no callers, tests, or CI references in this repo (its only mentions were its own docstring/argparse description), so this is a clean deletion.
Verification
grep -rn "badges" --include="*.py" --include="*.yml" --include="*.yaml" --include="*.toml" --include="*.cfg" --include="*.sh" --include=Makefile .— no references outside the deleted file.badges/*_test.py, no.github/references, no packaging (setup.py/pyproject.toml/MANIFEST.in) references.🤖 Generated with Claude Code
Greptile Summary
This PR removes
badges/reactions.py, a standalone script that generated a shields.io reaction-count badge SVG for theord-datarepository. The upstream consumer (ord-data) has already dropped the generated SVG, making this script dead code.setup.py,pyproject.toml,MANIFEST.in) referenced the file, so no install-time artifacts are affected.Confidence Score: 5/5
Pure deletion of a dead-code file with no callers, tests, or CI references — safe to merge as-is.
The change removes a single standalone script that was already abandoned by its only consumer. There is nothing left that depends on it, and no side effects from its removal.
No files require special attention.
Important Files Changed
Reviews (2): Last reviewed commit: "Merge branch 'main' into delete-reaction..." | Re-trigger Greptile