Skip to content

Delete unused reactions badge script#819

Merged
skearnes merged 2 commits into
mainfrom
delete-reactions-badge
Jun 19, 2026
Merged

Delete unused reactions badge script#819
skearnes merged 2 commits into
mainfrom
delete-reactions-badge

Conversation

@skearnes

@skearnes skearnes commented Jun 12, 2026

Copy link
Copy Markdown
Member

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 generated badges/reactions.svg was 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.
  • No 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 the ord-data repository. The upstream consumer (ord-data) has already dropped the generated SVG, making this script dead code.

  • The deleted file had no tests, no CI workflow references, and no callers elsewhere in the repository — the deletion is clean and complete.
  • No packaging manifests (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

Filename Overview
badges/reactions.py Deleted unused script that generated a reaction-count shields.io badge; confirmed to have no callers, tests, or CI references in the repo.

Reviews (2): Last reviewed commit: "Merge branch 'main' into delete-reaction..." | Re-trigger Greptile

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>
skearnes added a commit that referenced this pull request Jun 12, 2026
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>
@skearnes
skearnes requested a review from bdeadman June 12, 2026 02:32
@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.72%. Comparing base (b38513f) to head (5194341).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           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:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@skearnes
skearnes enabled auto-merge (squash) June 19, 2026 01:51
@skearnes
skearnes merged commit 41c46da into main Jun 19, 2026
35 of 42 checks passed
@skearnes
skearnes deleted the delete-reactions-badge branch June 19, 2026 02:01
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>
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