Prefetch the NEMAR sourcedata store in get_data() - #1148
Merged
Conversation
The store wired into loading by #1146 is only filled by an explicit download() call, so a plain get_data() on a fresh machine still fetched from the upstream host even with the provider pinned to 'nemar'. Fetch the requested subjects' sourcedata/ before loading, with the provider policy download() already implements: 'upstream' skips NEMAR, 'nemar' treats a failure as fatal, 'auto' warns per subject and leaves that subject to the dataset's own downloader. A non-empty store is trusted as-is and costs no network, preserving the warmed-store guarantee the end-to-end store test encodes.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
pooch.retrieve treats data_path's destination as a directory and stores <md5(url)>-<basename> inside it; loaders such as Rodrigues2017's then os.listdir() that directory. A store hit written as a plain file at the destination shadowed the wrapper directory and turned those loads into NotADirectoryError (Rodrigues2017, Cattan2019_PHMD, Cattan2019_VR in the 141-dataset sweep). The hit now lands inside the wrapper directory under pooch's unique name; verified live: Rodrigues2017 loads bit-identically from the store with zero upstream contact.
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
1 participant
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.
What
Follow-up to #1146, closing the last gap in its "not in scope" list (wiring the NEMAR sourcedata cache into
get_data): the store is only filled by an explicitdownload()call, so a plainget_data()on a fresh machine still fetched from the upstream host even with the provider pinned to"nemar"— andget_data()is what every evaluation and benchmark actually calls.get_data()now fetches the requested subjects'sourcedata/into the store before loading, reusingsourcedata_path()and the provider policydownload()already implements:"upstream"— skips NEMAR entirely (unchanged behavior)"nemar"— a NEMAR failure raises rather than silently reaching the host the caller opted out of"auto"— warns per subject and leaves that subject to the dataset's own downloaderA non-empty store is trusted as-is and costs no network, preserving the warmed-store guarantee
test_get_data_reads_the_nemar_store_end_to_endencodes (download(force_update=True)refreshes it).Verification
Live on Nakanishi2015 (fresh caches, sample arrays hashed):
get_data()get_data()a835e50de5aa42a9download()→get_data()a835e50de5aa42a9get_data(), provider=nemara835e50de5aa42a9get_data(), provider=nemara835e50de5aa42a9Bit-identical data in all modes; with this PR the upstream host is never contacted.
pytest moabb/tests/test_download.py→ 209 passed (5 new: provider semantics per mode, no-nemar_id skip,get_dataprefetches exactly the requested subjects)ruff check/ruff formatclean