Key the dataset cache on the config and source files - #1265
Merged
Merged
Conversation
The dataset cache key covered only {root, tables, dataset_name, dev}.
Rewriting a source file at the same path, or editing the YAML config,
silently reused the stale cached event table.
- The key now also includes a SHA-256 of the YAML config and, for each
requested table, [file, size, mtime_ns] of every source file it reads:
the table file, its .csv/.csv.gz twin, joined files, glob matches, and
files under a Parquet directory. Contents are not hashed, so startup
stays cheap on large sources; URLs are keyed by the URL alone, so no
network calls are made at init. Datasets without a config are unchanged.
- MEDSDataset builds on the base key and gets this too.
- tests/core/test_dataset_cache_key.py: a rewritten source is reloaded
(3 -> 5 patients), an unchanged rerun reuses the cache, an edited
config and a changed mtime give a new cache.
- docs/api/datasets.rst: what "same configuration" means for the cache.
- examples/dataset_cache_refresh.py.
Existing caches get a new key once after upgrading; old cache folders are
left in place.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
Problem
The dataset cache key (
BaseDataset._init_cache_dir) covered only{root, tables, dataset_name, dev}. So:global_event_df;A downstream EHR project hit both and had to delete caches by hand.
Change
The key now also includes:
DatasetConfig.model_dump);[file, size, mtime_ns]of every source file it reads: the table file and its.csv/.csv.gztwin, joined files, glob matches, and the files under a Parquet directory (MEDS shards).Design choices:
make. Startup stays cheap for multi-GB sources. Copying files with new timestamps triggers a rebuild, which errs on the safe side.MEDSDatasetbuilds on the base key, and the FHIR dataset already hashes its YAML in its own key.Scope
This is the dataset half. The task cache key (task code and attributes) is being reworked in #1207, so this PR doesn't touch it.
Upgrade note
Existing caches get a new key, so they're rebuilt once after upgrading. Old cache folders are left in place; the docs say they can be deleted.
Tests, docs, example
New
tests/core/test_dataset_cache_key.py, on a real CSV and YAML in a temp folder:Three of these fail on master.
Existing base-dataset, caching and MEDS tests pass.
docs/api/datasets.rst("How PyHealth Loads Data") explains what "the same configuration" means now.New
examples/dataset_cache_refresh.py: v1 has 2 patients; after rewriting the file, v2 has 4 patients in a fresh cache, and an unchanged rerun reuses it.Full core suite:
Ran 1385 tests … OK (skipped=76).tools/check_pr_rules.pypasses.This PR and #1264 (the default cache warning) edit different parts of
docs/api/datasets.rst, so they merge in either order.🤖 Generated with Claude Code