Fix Schirrmeister2017 re-downloading every recording - #1132
Open
adityasingh2400 wants to merge 1 commit into
Open
Fix Schirrmeister2017 re-downloading every recording#1132adityasingh2400 wants to merge 1 commit into
adityasingh2400 wants to merge 1 commit into
Conversation
data_path moved each freshly downloaded EDF out of the directory owned by data_dl and into MNE-schirrmeister2017-data/<train|test>/. That left the download cache empty, so the next call fetched the whole multi-gigabyte recording again, and the refetched copy was then abandoned in place because the destination already existed. data_path now returns the path reported by data_dl and only reads from the old layout when a file is already there, so an existing local copy is still reused and never downloaded twice. Fixes NeuroTechX#851
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.
Schirrmeister2017.data_pathdownloads each subject's train and test EDF throughmoabb.datasets.download.data_dl, then moves the file out of the directorydata_dlowns (MNE-schirrmeister2017-data/robintibor/high-gamma-dataset/raw/master/data/<train|test>/) and intoMNE-schirrmeister2017-data/<train|test>/.data_dlderives its destination from the URL, so on the next call it looks in the directory the file was moved out of, finds nothing, and fetches the whole recording again. The refetched copy is then abandoned where it landed, because the move is guarded byif not os.path.exists(new_path)and the relocated file is already there. The net effect is one redundant full re-download of every file plus two permanent copies of a multi-gigabyte recording on disk. This is what @toncho11 reported in #851.The relocation was cosmetic.
data_dlalready keeps the train and test files in separate directories, since it mirrors the URL path, so the extratrain/andtest/folders were not adding any separation that was missing.data_pathnow returns the pathdata_dlreports and never writes to the old location. It still reads from it first, so anyone with data already on disk from a previous MOABB version keeps using that copy instead of downloading ~50 GB again. Withforce_update=Truethe stale relocated file is removed before the refresh, otherwise it would shadow the newly downloaded one on the following call.Tested with two regression tests in
moabb/tests/test_dataset_fixes.pythat stubdata_dlwith a stand-in reproducing its caching contract, so no network access is involved.test_schirrmeister2017_does_not_redownloadcallsdata_paththree times and asserts two downloads and two files on disk (ondevelopit observes four downloads and four files).test_schirrmeister2017_reuses_relocated_filespre-populates the old layout and fails if anything is downloaded at all. Both fail before the change and pass after it, andpre-commit run --filesis clean on the three touched files.Fixes #851