From d668f5b8807420aaa4dd3c22f73dc9d19a81fa4a Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 06:59:34 -0700 Subject: [PATCH 1/7] Fix Lee2024 silent file drops and RestingStateToP300Adapter default Lee2024 (gh-1142), verified against the upstream git tree: subject 8's Doorlock files carry the unpadded id (sub8_*), so a 404 under the padded name retries unpadded; combined and per-block training files coexist in mixtures the configs do not capture, so both forms are requested and a 404 sorts out which exist; param.mat (shipped for all AirConditioner subjects) is requested; and any non-404 failure now raises instead of leaving a silently incomplete directory. RestingStateToP300Adapter (gh-1143 item 3): events=None now means all of the dataset's events, matching how every in-repo usage constructs it, so the adapter can be built with defaults. --- moabb/datasets/lee2024.py | 31 +++++++++++++++--------------- moabb/paradigms/resting_state.py | 2 ++ moabb/tests/test_datasets.py | 33 ++++++++++++++++++++++++++++++++ moabb/tests/test_paradigms.py | 3 +++ 4 files changed, 54 insertions(+), 15 deletions(-) diff --git a/moabb/datasets/lee2024.py b/moabb/datasets/lee2024.py index 8afa52911a..0747236fd7 100644 --- a/moabb/datasets/lee2024.py +++ b/moabb/datasets/lee2024.py @@ -456,17 +456,16 @@ def data_path( for i in range(1, 31): files_to_dl.append(f"{subj_str}_Testing{i}.mat") - # Training blocks (per-block). - if config["has_training"] and not config["training_combined"]: - for i in range(1, 51): - files_to_dl.append(f"{subj_str}_Training{i}.mat") - - # Combined training. - if config["has_training"] and config["training_combined"]: + # Training blocks. Upstream ships combined and per-block files in + # mixtures the configs do not capture (AirConditioner is "combined" + # yet six subjects also have Training1..50), so request both forms + # and let the 404 skip sort out which exist for this subject (gh-1142). + if config["has_training"]: files_to_dl.append(f"{subj_str}_Training.mat") + files_to_dl.extend(f"{subj_str}_Training{i}.mat" for i in range(1, 51)) - # Calibration signal. - files_to_dl.append("cal_sig.mat") + # Calibration signal and recording parameters. + files_to_dl += ["cal_sig.mat", "param.mat"] subj_dir.mkdir(parents=True, exist_ok=True) @@ -474,19 +473,21 @@ def data_path( local = subj_dir / fname if local.exists() and not force_update: continue - dir_name = config["dir_name"] - url = f"{_GITHUB_RAW}/{dir_name}/Dat_{subj_str}/{fname}" - log.info("Downloading %s ...", fname) - try: + # Subject 8's files carry the UNPADDED id upstream + # (Dat_sub08/sub8_Testing1.mat), so a 404 under the padded name + # retries unpadded. Any other failure now raises instead of + # leaving a silently incomplete directory (gh-1142). + for name in dict.fromkeys([fname, fname.replace(subj_str, f"sub{subject}")]): + url = f"{_GITHUB_RAW}/{config['dir_name']}/Dat_{subj_str}/{name}" resp = _requests.get(url, stream=True, timeout=120) if resp.status_code == 404: continue resp.raise_for_status() + log.info("Downloading %s ...", name) with open(local, "wb") as fout: for chunk in resp.iter_content(chunk_size=8192): fout.write(chunk) - except Exception as e: - log.warning("Download failed for %s: %s", fname, e) + break return str(subj_dir) diff --git a/moabb/paradigms/resting_state.py b/moabb/paradigms/resting_state.py index 87079622a3..1837a462ce 100644 --- a/moabb/paradigms/resting_state.py +++ b/moabb/paradigms/resting_state.py @@ -72,6 +72,8 @@ def __init__( ) def used_events(self, dataset): + if self.events is None: + return dict(dataset.event_id) return {ev: dataset.event_id[ev] for ev in self.events} def is_valid(self, dataset): diff --git a/moabb/tests/test_datasets.py b/moabb/tests/test_datasets.py index 2e4cdf6225..0808c6664d 100644 --- a/moabb/tests/test_datasets.py +++ b/moabb/tests/test_datasets.py @@ -1755,3 +1755,36 @@ def test_constructor_summary_table_cross_ref(dataset_cls): warnings.warn( f"{name} summary CSV mismatch: {'; '.join(mismatches)}", stacklevel=1 ) + + +def test_lee2024_data_path_requests_every_upstream_form(tmp_path, monkeypatch): + """gh-1142: padded AND unpadded names, both training forms, param.mat; + a non-404 failure raises instead of leaving a silently incomplete dir.""" + import requests + + from moabb.datasets import Lee2024_DL + + seen = [] + + class _NotFound: + status_code = 404 + + monkeypatch.setattr(requests, "get", lambda url, **k: seen.append(url) or _NotFound()) + dataset = Lee2024_DL() + dataset.data_path(8, path=str(tmp_path)) + urls = "\n".join(seen) + assert "sub08_Testing1.mat" in urls # padded first + assert "sub8_Testing1.mat" in urls # unpadded retry (upstream layout) + assert "sub08_Training.mat" in urls # combined form + assert "sub08_Training50.mat" in urls # per-block form + assert "param.mat" in urls + + class _Boom: + status_code = 500 + + def raise_for_status(self): + raise requests.HTTPError("500") + + monkeypatch.setattr(requests, "get", lambda url, **k: _Boom()) + with pytest.raises(requests.HTTPError): + dataset.data_path(1, path=str(tmp_path)) diff --git a/moabb/tests/test_paradigms.py b/moabb/tests/test_paradigms.py index ae6bbc5897..56dc09c69c 100644 --- a/moabb/tests/test_paradigms.py +++ b/moabb/tests/test_paradigms.py @@ -769,6 +769,9 @@ def test_RestingState_paradigm(self): def test_RestingState_default_values(self): paradigm = RestingStateToP300Adapter() + # gh-1143: events=None must mean "all of the dataset's events" + dataset = FakeDataset(paradigm="rstate", event_list=["Open", "Close"]) + assert paradigm.used_events(dataset) == dataset.event_id assert paradigm.tmin == 10 assert paradigm.tmax == 50 assert paradigm.fmin == 1 From a6456aa9fd390a34ceb4a45969b2c4b92876b60b Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 07:03:12 -0700 Subject: [PATCH 2/7] Fix BNCI2022-001 event/interval declaration causing huge epoch allocation The dataset declared all four trigger codes (trajectory_start, waypoint_miss, waypoint_hit, trajectory_end) as trial events under the 90 s trajectory interval. The waypoint and trajectory-end codes are instantaneous point markers, and the loader additionally annotated every non-zero sample of each held trigger pulse, so default paradigm processing tried to epoch 33k+ events of 90 s each per subject (~364 GiB, gh-1143 defect 1). - Declare only trajectory_start as the trial event, matching the ~90 s trajectory interval (32 trials per subject). - Annotate only the onset of each trigger pulse (rising edge / value change) instead of every non-zero sample, so each real event yields a single annotation. Waypoint hit/miss and trajectory-end markers remain available as annotations on the loaded raws. - Add synthetic-raw unit tests covering the declaration, the pulse edge-detection, and the boundedness of default trial derivation. --- moabb/datasets/bnci/bnci_2022_001.py | 34 +++++++--- moabb/tests/test_bnci.py | 97 +++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 10 deletions(-) diff --git a/moabb/datasets/bnci/bnci_2022_001.py b/moabb/datasets/bnci/bnci_2022_001.py index 4f8fb3575a..d5e400a8b6 100644 --- a/moabb/datasets/bnci/bnci_2022_001.py +++ b/moabb/datasets/bnci/bnci_2022_001.py @@ -299,8 +299,14 @@ def _convert_run_001_2022( 255: "trajectory_end", } - # Find non-zero trigger positions - event_indices = np.where(trigger != 0)[0] + # The hardware trigger is an 8-bit signal sampled continuously, so one + # event pulse can stay non-zero over several consecutive samples. + # Annotate only the onset of each pulse (a rising edge or a change of + # value); annotating every non-zero sample would turn each pulse into + # dozens of duplicated events. + trigger = np.asarray(trigger) + previous = np.concatenate(([0], trigger[:-1])) + event_indices = np.flatnonzero((trigger != 0) & (trigger != previous)) if len(event_indices) > 0: event_times = event_indices / sfreq event_values = trigger[event_indices].astype(int) @@ -381,6 +387,15 @@ class BNCI2022_001(BNCIBaseDataset): - waypoint_hit (48): Drone successfully passed through waypoint - trajectory_end (255): End of trajectory (3s after final waypoint) + Only ``trajectory_start`` is declared as a trial event (``events``), matching + the ~90 second trajectory ``interval``: paradigms therefore epoch the 32 + trajectories per subject. The waypoint and trajectory-end codes mark + instantaneous point events (roughly a thousand per subject), so epoching + them with the 90 s trial window is not meaningful. They are still annotated + on the raw data returned by the loader; to analyse them, epoch the raw + annotations directly or pass a custom ``process_pipeline`` to + :meth:`~moabb.datasets.base.BaseDataset.get_data`. + **Data Organization** - 1 session per subject (offline data only, online sessions not included) @@ -661,12 +676,15 @@ def __init__(self, subjects=None, sessions=None, *, return_all_modalities=False) super().__init__( subjects=list(range(1, 14)), sessions_per_subject=1, - events={ - "trajectory_start": 1, - "waypoint_miss": 16, - "waypoint_hit": 48, - "trajectory_end": 255, - }, + # Only the trajectory itself is a trial: each of the 32 trajectories + # lasts ~90 seconds and starts at trigger code 1. The other trigger + # codes (waypoint_miss=16, waypoint_hit=48, trajectory_end=255) are + # instantaneous point markers occurring ~1000 times per subject; + # epoching them with the 90 s trial interval would produce hundreds + # of hours of overlapping epochs per subject. They are therefore not + # declared as trial events, but remain available as annotations on + # the loaded raw data (see ``_convert_run_001_2022``). + events={"trajectory_start": 1}, code="BNCI2022-001", interval=[0, 90], # Approximately 90 seconds per trajectory paradigm="imagery", # For compatibility diff --git a/moabb/tests/test_bnci.py b/moabb/tests/test_bnci.py index e415bad2ae..cc6eac39da 100644 --- a/moabb/tests/test_bnci.py +++ b/moabb/tests/test_bnci.py @@ -1,10 +1,12 @@ from types import SimpleNamespace import numpy as np +from scipy.io import savemat -from moabb.datasets import BNCI2014_001, BNCI2014_008 +from moabb.datasets import BNCI2014_001, BNCI2014_008, BNCI2022_001 from moabb.datasets.bnci.base import _BNCI_ARTIFACT_ANNOTATION_DESCRIPTION, _convert_run -from moabb.datasets.preprocessing import _is_preserved_annotation +from moabb.datasets.bnci.bnci_2022_001 import _convert_run_001_2022 +from moabb.datasets.preprocessing import SetRawAnnotations, _is_preserved_annotation def _fake_mi_run(): @@ -79,6 +81,97 @@ def test_bnci_artifact_markers_survive_event_rederivation(): assert _is_preserved_annotation(description) +def _fake_2022_001_mat(path, sfreq=128, n_trajectories=3, pulse_samples=13): + """Write a synthetic BNCI2022-001 task MAT file and return layout info. + + Mimics the public release structure (``EEG``, ``EOG``, ``Trigger``, + ``Header``): each ~90 s trajectory starts with a trigger pulse of code 1, + contains 4 waypoint pulses (codes 48/16 alternating) and ends with a code + 255 pulse. As in the real recordings, the hardware trigger holds each code + for several consecutive samples (``pulse_samples``). + """ + traj_samples = 92 * sfreq # ~90 s trajectory + 2 s gap + n_samples = n_trajectories * traj_samples + trigger = np.zeros(n_samples) + for k in range(n_trajectories): + t0 = k * traj_samples + trigger[t0 : t0 + pulse_samples] = 1 + for w in range(4): + p = t0 + (w + 1) * 10 * sfreq + trigger[p : p + pulse_samples] = 48 if w % 2 == 0 else 16 + end = t0 + 90 * sfreq + trigger[end : end + pulse_samples] = 255 + rng = np.random.RandomState(42) + savemat( + path, + { + "EEG": rng.standard_normal((n_samples, 64)) * 10.0, # microvolts + "EOG": rng.standard_normal((n_samples, 3)) * 10.0, + "Trigger": trigger, + "Header": {"fs": float(sfreq)}, + }, + ) + return n_trajectories + + +def _load_fake_2022_001_raw(tmp_path): + mat_path = str(tmp_path / "s1w.mat") + n_traj = _fake_2022_001_mat(mat_path) + ch_names = [f"EEG{i:02d}" for i in range(1, 65)] + ["EOG1", "EOG2", "EOG3"] + ch_types = ["eeg"] * 64 + ["eog"] * 3 + raw = _convert_run_001_2022(mat_path, ch_names, ch_types, subject_id=1) + return raw, n_traj + + +def test_bnci2022_001_declares_only_trajectory_trials(): + """Point events must not be declared as 90 s trials. + + The waypoint hit/miss and trajectory-end triggers are instantaneous + markers occurring ~1000 times per subject; combined with the 90 s trial + interval they made default paradigm epoching try to allocate hundreds of + GiB (gh-1143, defect 1). Only the ~90 s trajectory is a trial. + """ + dataset = BNCI2022_001() + assert dataset.event_id == {"trajectory_start": 1} + assert dataset.interval == [0, 90] + + +def test_bnci2022_001_trigger_pulses_annotated_once(tmp_path): + """Each held trigger pulse must yield exactly one annotation. + + The 8-bit hardware trigger holds each event code over several consecutive + samples; annotating every non-zero sample multiplied each real event into + dozens of duplicates (33k+ events per subject on develop). + """ + raw, n_traj = _load_fake_2022_001_raw(tmp_path) + desc = raw.annotations.description + assert np.sum(desc == "trajectory_start") == n_traj + assert np.sum(desc == "waypoint_hit") == 2 * n_traj + assert np.sum(desc == "waypoint_miss") == 2 * n_traj + assert np.sum(desc == "trajectory_end") == n_traj + + +def test_bnci2022_001_default_epoching_is_bounded(tmp_path): + """Default trial derivation must stay within the recording's duration. + + Applies the dataset's own event_id/interval the way both + ``BaseDataset.get_data`` and the paradigm pipelines do (via + ``SetRawAnnotations``) and checks that the resulting trials are the + n_trajectories 90 s trajectories, not thousands of overlapping 90 s + epochs anchored on instantaneous waypoint markers. + """ + dataset = BNCI2022_001() + raw, n_traj = _load_fake_2022_001_raw(tmp_path) + transform = SetRawAnnotations(dataset.event_id, interval=tuple(dataset.interval)) + raw = transform.transform(raw) + + trial_annotations = raw.annotations + assert set(trial_annotations.description) == {"trajectory_start"} + assert len(trial_annotations) == n_traj + # total epoched time cannot exceed the recording length + assert trial_annotations.duration.sum() <= raw.times[-1] + + def test_bnci2014_001_metadata(): """Test metadata for BNCI2014-001.""" dataset = BNCI2014_001() From 8feaa6a5b287211c449b8b46d432b0200c0ee3b7 Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 07:05:05 -0700 Subject: [PATCH 3/7] Fix Kaneshiro2015 paradigm tag so its declared paradigm accepts it Kaneshiro2015 declared paradigm="p300" but its six events are object categories (human_body, human_face, animal_body, animal_face, fruit_vegetable, inanimate_object) with no Target/NonTarget, so P300.is_valid/used_events rejected the dataset for every subject. Retag it as paradigm="imagery" (in both the runtime declaration and the experiment metadata) so the n-class paradigms accept it and resolve all six classes, following the existing convention for non-MI multiclass datasets (BNCI2022_001, Shin2017B). Add a regression test asserting MotorImagery/Imagery accept the dataset and resolve its six events, and that P300 does not. --- moabb/datasets/kaneshiro2015.py | 8 ++++++-- moabb/tests/test_dataset_fixes.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/moabb/datasets/kaneshiro2015.py b/moabb/datasets/kaneshiro2015.py index 2c7b8de805..259cea1817 100644 --- a/moabb/datasets/kaneshiro2015.py +++ b/moabb/datasets/kaneshiro2015.py @@ -98,7 +98,7 @@ class Kaneshiro2015(BaseDataset): ), experiment=ExperimentMetadata( events=dict(_EVENTS), - paradigm="p300", + paradigm="imagery", # 6-class visual ERP, no Target/NonTarget n_classes=6, class_labels=list(_EVENTS.keys()), trial_duration=0.496, @@ -143,7 +143,11 @@ def __init__(self, subjects=None, sessions=None): events=dict(_EVENTS), code="Kaneshiro2015", interval=[0, 0.496], - paradigm="p300", + # Six object categories with no Target/NonTarget structure, so + # the P300 paradigm cannot process this dataset. The "imagery" + # tag routes it to the n-class paradigms (as done for other + # non-MI multiclass datasets, e.g. BNCI2022_001). + paradigm="imagery", doi=_DOI, selected_subjects=subjects, selected_sessions=sessions, diff --git a/moabb/tests/test_dataset_fixes.py b/moabb/tests/test_dataset_fixes.py index f9a3cdfc2b..f01100a2a9 100644 --- a/moabb/tests/test_dataset_fixes.py +++ b/moabb/tests/test_dataset_fixes.py @@ -14,6 +14,7 @@ from moabb.datasets.bnci.bnci_2020 import _convert_attention_shift from moabb.datasets.braininvaders import BI2015b from moabb.datasets.hefmi_ich2025 import HefmiIch2025 +from moabb.datasets.kaneshiro2015 import Kaneshiro2015 from moabb.datasets.kojima2024a import Kojima2024A from moabb.datasets.mainsah2025 import _parse_manifest from moabb.datasets.schirrmeister2017 import Schirrmeister2017 @@ -301,3 +302,33 @@ def test_schirrmeister2017_reuses_relocated_files(tmp_path: Path, monkeypatch): dataset = Schirrmeister2017() assert dataset.data_path(1, path=str(tmp_path)) == relocated + + +def test_kaneshiro2015_valid_for_declared_paradigm(): + """Kaneshiro2015 must be accepted by the paradigm it declares. + + The dataset has six object categories and no Target/NonTarget + events, so it cannot satisfy the P300 paradigm; it is tagged + "imagery" to route it to the n-class paradigms (like BNCI2022_001). + """ + from moabb.paradigms import P300, Imagery, MotorImagery + + dataset = Kaneshiro2015() + + assert dataset.paradigm == "imagery" + assert dataset.metadata.experiment.paradigm == dataset.paradigm + + for paradigm in (MotorImagery(), Imagery(), MotorImagery(n_classes=6)): + assert paradigm.is_valid(dataset) + used = paradigm.used_events(dataset) + assert used == { + "human_body": 1, + "human_face": 2, + "animal_body": 3, + "animal_face": 4, + "fruit_vegetable": 5, + "inanimate_object": 6, + } + + # The old declaration was broken: P300 requires Target/NonTarget. + assert not P300().is_valid(dataset) From 88cbb177662759440007ad703633b3547d8d8b67 Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 07:17:19 -0700 Subject: [PATCH 4/7] Move Kaneshiro2015 catalog row and docs entry to imagery --- docs/source/api.rst | 2 +- moabb/datasets/summary_imagery.csv | 1 + moabb/datasets/summary_p300.csv | 1 - 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/api.rst b/docs/source/api.rst index 8c1b06c0ad..eee7910331 100644 --- a/docs/source/api.rst +++ b/docs/source/api.rst @@ -67,6 +67,7 @@ Motor Imagery Datasets BNCI2025_001 BNCI2025_002 Cho2017 + Kaneshiro2015 Dreyer2023 Dreyer2023A Dreyer2023B @@ -176,7 +177,6 @@ ERP/P300 Datasets Lee2021Mobile_ERP Chailloux2020 GuttmannFlury2025_P300 - Kaneshiro2015 Lee2024_AC Lee2024_BS Lee2024_DL diff --git a/moabb/datasets/summary_imagery.csv b/moabb/datasets/summary_imagery.csv index 8680cf3123..77baf6769b 100644 --- a/moabb/datasets/summary_imagery.csv +++ b/moabb/datasets/summary_imagery.csv @@ -7,6 +7,7 @@ BNCI2014_004,9,3,2,360,4.5,250,5,1,32400 BNCI2015_001,12,13,2,200,5.0,512,3,1,14400 BNCI2015_004,9,30,5,80,7.0,256,2,1,7200 Cho2017,52,64,2,100,3.0,512,1,1,9800 +Kaneshiro2015,10,124,6,864,0.496,62.5,1,1,5184 Lee2019_MI,54,62,2,100,4.0,1000,2,1,11000 GrosseWentrup2009,10,128,2,150,7.0,500,1,1,3000 Schirrmeister2017,14,128,4,120,4.0,500,1,2,13440 diff --git a/moabb/datasets/summary_p300.csv b/moabb/datasets/summary_p300.csv index 296d890563..3143f09b78 100644 --- a/moabb/datasets/summary_p300.csv +++ b/moabb/datasets/summary_p300.csv @@ -64,7 +64,6 @@ Lee2024_BS,14,31,varies NT / T,1.0,500,1 Lee2024_AC,10,25,varies NT / T,1.0,500,1 Zheng2020,14,62,168 T / 4032 NT,1.0,1000,2 Zhang2025,15,57,varies T / NT,1.0,1000,4 -Kaneshiro2015,10,124,5184 (6 classes),0.496,62.5,1 Simoes2020,15,8,varies NT / T,1.0,250,7 Speier2017,10,32,~1200 per run,1.0,256,2 BCIComp2020WalkingERP,15,46,240 NT / 60 T,1.0,100,1 From 528cffff8feeee3bde6b6e527eda015e8d9061e5 Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 07:18:27 -0700 Subject: [PATCH 5/7] Changelog for the catalogue defect fixes --- docs/source/whats_new.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/source/whats_new.rst b/docs/source/whats_new.rst index b2f19cf366..d0d37c1f99 100644 --- a/docs/source/whats_new.rst +++ b/docs/source/whats_new.rst @@ -61,6 +61,10 @@ Requirements Bugs ~~~~ +- Fix :class:`moabb.datasets.BNCI2022_001` epoching its instantaneous waypoint markers as 90-second trials: only ``trajectory_start`` is a trial event now (32 bounded, non-overlapping 90 s epochs per subject), and the loader annotates each trigger pulse once (rising edge) instead of once per held sample, which had inflated subject 1 to 33,114 annotations and a 364 GiB allocation. Waypoint and trajectory-end markers stay available as annotations on the loaded raws (:gh:`1143` by `Bruno Aristimunha`_) +- Retag :class:`moabb.datasets.Kaneshiro2015` from ``p300`` to ``imagery`` so its declared default paradigm accepts it: its six visual object categories carry no Target/NonTarget pair, so the P300 paradigm rejected every subject. The catalog row and documentation grouping move with it (:gh:`1143` by `Bruno Aristimunha`_) +- Allow :class:`moabb.paradigms.RestingStateToP300Adapter` to be constructed with defaults: ``events=None`` now means all of the dataset's events, matching how every in-repo usage builds it (:gh:`1143` by `Bruno Aristimunha`_) +- Fix :class:`moabb.datasets.Lee2024` silently dropping 345 upstream files, verified against the upstream git tree: subject 8's Doorlock files carry the unpadded id (``sub8_*``) and are now retried under that name after a 404; combined and per-block training files coexist in mixtures the configs do not capture, so both forms are requested; ``param.mat`` is fetched; and any non-404 failure now raises instead of leaving a silently incomplete directory (:gh:`1142` by `Bruno Aristimunha`_) - Prefetch the NEMAR sourcedata store inside :meth:`moabb.datasets.base.BaseDataset.get_data`: the store introduced in :gh:`1146` was only filled by an explicit :meth:`~moabb.datasets.base.BaseDataset.download` call, so a plain ``get_data()`` on a fresh machine still fetched from the upstream host even with the provider pinned to ``"nemar"``. The requested subjects' ``sourcedata/`` is now fetched 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. Also fix the store fill in the deprecated :func:`moabb.datasets.download.data_path`: ``pooch.retrieve`` treats its destination as a *directory* holding ``-``, and loaders such as :class:`moabb.datasets.Rodrigues2017`'s ``os.listdir()`` it, so a store hit written as a plain file at the destination raised ``NotADirectoryError``; the hit now lands inside the wrapper directory under pooch's unique name (by `Bruno Aristimunha`_) - Wire the NEMAR sourcedata store into loading (:gh:`1147`): ``data_dl`` and the deprecated ``data_path`` now serve a requested file from the dataset's ``NEMAR//sourcedata/`` store before consulting the URL-derived layout, probing the store by the trailing segments of the URL path since it keeps the upstream filenames. ``dataset.download()`` followed by ``get_data()`` therefore no longer re-contacts the upstream host -- verified live against ``Schirrmeister2017``, whose host is currently unreachable. The URL-derived trees remain as lookups so nothing already downloaded is fetched again; ``force_update`` still refetches upstream and pinning the provider to ``"upstream"`` opts loading out of the store (:gh:`1146` by `Bruno Aristimunha`_) - Store :class:`moabb.datasets.ErpCore2021` as the single combined BIDS dataset it is, with components separated by the ``task-`` entity in one shared ``MNE-erpcore2021-data`` root, instead of seven standalone per-component BIDS datasets; likewise store Dreyer2023 in one shared ``MNE-dreyer2023-data`` root, since the A/B/C classes only select subject ranges of one globally numbered dataset. Pre-existing downloads in the legacy separated layouts are still read without re-fetching (:gh:`1146` by `Bruno Aristimunha`_) From 8ce6a84fbafd399e8541cd90d99e59b7f988a356 Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 07:33:45 -0700 Subject: [PATCH 6/7] Simplify the catalogue fixes Lee2024: replace the filename-guessing download loop with a real inventory -- the NEMAR deposit's provenance manifest when the sourcedata store is in use (the NEMAR path then never contacts the upstream host), or the upstream git tree otherwise -- and route each file through the shared data_dl, which also serves it from the NEMAR store. Removes the private requests loop, the per-file 404 probing and the padded/unpadded retry storm. BNCI2022-001: one-expression rising-edge detection; trim a comment that restated the class docstring. RestingStateToP300Adapter: fold the events=None default into the existing comprehension. Test files: fold redundant asserts, one-line docstrings, assert against dataset.event_id instead of re-spelled literals. --- docs/source/whats_new.rst | 2 +- moabb/datasets/bnci/bnci_2022_001.py | 23 ++---- moabb/datasets/kaneshiro2015.py | 6 +- moabb/datasets/lee2024.py | 102 +++++++++++++++------------ moabb/paradigms/resting_state.py | 4 +- moabb/tests/test_bnci.py | 31 ++------ moabb/tests/test_dataset_fixes.py | 22 +----- moabb/tests/test_datasets.py | 75 ++++++++++++-------- 8 files changed, 122 insertions(+), 143 deletions(-) diff --git a/docs/source/whats_new.rst b/docs/source/whats_new.rst index d0d37c1f99..29757fa9ea 100644 --- a/docs/source/whats_new.rst +++ b/docs/source/whats_new.rst @@ -64,7 +64,7 @@ Bugs - Fix :class:`moabb.datasets.BNCI2022_001` epoching its instantaneous waypoint markers as 90-second trials: only ``trajectory_start`` is a trial event now (32 bounded, non-overlapping 90 s epochs per subject), and the loader annotates each trigger pulse once (rising edge) instead of once per held sample, which had inflated subject 1 to 33,114 annotations and a 364 GiB allocation. Waypoint and trajectory-end markers stay available as annotations on the loaded raws (:gh:`1143` by `Bruno Aristimunha`_) - Retag :class:`moabb.datasets.Kaneshiro2015` from ``p300`` to ``imagery`` so its declared default paradigm accepts it: its six visual object categories carry no Target/NonTarget pair, so the P300 paradigm rejected every subject. The catalog row and documentation grouping move with it (:gh:`1143` by `Bruno Aristimunha`_) - Allow :class:`moabb.paradigms.RestingStateToP300Adapter` to be constructed with defaults: ``events=None`` now means all of the dataset's events, matching how every in-repo usage builds it (:gh:`1143` by `Bruno Aristimunha`_) -- Fix :class:`moabb.datasets.Lee2024` silently dropping 345 upstream files, verified against the upstream git tree: subject 8's Doorlock files carry the unpadded id (``sub8_*``) and are now retried under that name after a 404; combined and per-block training files coexist in mixtures the configs do not capture, so both forms are requested; ``param.mat`` is fetched; and any non-404 failure now raises instead of leaving a silently incomplete directory (:gh:`1142` by `Bruno Aristimunha`_) +- Fix :class:`moabb.datasets.Lee2024` silently dropping 345 upstream files. ``data_path`` no longer guesses filenames and skips failures: it downloads a real inventory -- the NEMAR deposit's provenance manifest when the sourcedata store is in use (so the NEMAR path never contacts the upstream host), or the upstream git tree otherwise -- through the shared :func:`moabb.datasets.download.data_dl`, which also serves the files from the NEMAR store. Subject 8's unpadded upstream names (``sub8_*``) are normalized to the padded names the loader reads, mixtures of combined and per-block training files and ``param.mat`` are covered by the inventory, and any download failure now raises instead of leaving a silently incomplete directory (:gh:`1142` by `Bruno Aristimunha`_) - Prefetch the NEMAR sourcedata store inside :meth:`moabb.datasets.base.BaseDataset.get_data`: the store introduced in :gh:`1146` was only filled by an explicit :meth:`~moabb.datasets.base.BaseDataset.download` call, so a plain ``get_data()`` on a fresh machine still fetched from the upstream host even with the provider pinned to ``"nemar"``. The requested subjects' ``sourcedata/`` is now fetched 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. Also fix the store fill in the deprecated :func:`moabb.datasets.download.data_path`: ``pooch.retrieve`` treats its destination as a *directory* holding ``-``, and loaders such as :class:`moabb.datasets.Rodrigues2017`'s ``os.listdir()`` it, so a store hit written as a plain file at the destination raised ``NotADirectoryError``; the hit now lands inside the wrapper directory under pooch's unique name (by `Bruno Aristimunha`_) - Wire the NEMAR sourcedata store into loading (:gh:`1147`): ``data_dl`` and the deprecated ``data_path`` now serve a requested file from the dataset's ``NEMAR//sourcedata/`` store before consulting the URL-derived layout, probing the store by the trailing segments of the URL path since it keeps the upstream filenames. ``dataset.download()`` followed by ``get_data()`` therefore no longer re-contacts the upstream host -- verified live against ``Schirrmeister2017``, whose host is currently unreachable. The URL-derived trees remain as lookups so nothing already downloaded is fetched again; ``force_update`` still refetches upstream and pinning the provider to ``"upstream"`` opts loading out of the store (:gh:`1146` by `Bruno Aristimunha`_) - Store :class:`moabb.datasets.ErpCore2021` as the single combined BIDS dataset it is, with components separated by the ``task-`` entity in one shared ``MNE-erpcore2021-data`` root, instead of seven standalone per-component BIDS datasets; likewise store Dreyer2023 in one shared ``MNE-dreyer2023-data`` root, since the A/B/C classes only select subject ranges of one globally numbered dataset. Pre-existing downloads in the legacy separated layouts are still read without re-fetching (:gh:`1146` by `Bruno Aristimunha`_) diff --git a/moabb/datasets/bnci/bnci_2022_001.py b/moabb/datasets/bnci/bnci_2022_001.py index d5e400a8b6..e79c76cbda 100644 --- a/moabb/datasets/bnci/bnci_2022_001.py +++ b/moabb/datasets/bnci/bnci_2022_001.py @@ -299,14 +299,11 @@ def _convert_run_001_2022( 255: "trajectory_end", } - # The hardware trigger is an 8-bit signal sampled continuously, so one - # event pulse can stay non-zero over several consecutive samples. - # Annotate only the onset of each pulse (a rising edge or a change of - # value); annotating every non-zero sample would turn each pulse into - # dozens of duplicated events. - trigger = np.asarray(trigger) - previous = np.concatenate(([0], trigger[:-1])) - event_indices = np.flatnonzero((trigger != 0) & (trigger != previous)) + # The trigger is sampled continuously, so a pulse spans several + # samples; annotate only its onset (rising edge or value change). + event_indices = np.flatnonzero( + (trigger != 0) & (np.diff(trigger, prepend=0) != 0) + ) if len(event_indices) > 0: event_times = event_indices / sfreq event_values = trigger[event_indices].astype(int) @@ -676,14 +673,8 @@ def __init__(self, subjects=None, sessions=None, *, return_all_modalities=False) super().__init__( subjects=list(range(1, 14)), sessions_per_subject=1, - # Only the trajectory itself is a trial: each of the 32 trajectories - # lasts ~90 seconds and starts at trigger code 1. The other trigger - # codes (waypoint_miss=16, waypoint_hit=48, trajectory_end=255) are - # instantaneous point markers occurring ~1000 times per subject; - # epoching them with the 90 s trial interval would produce hundreds - # of hours of overlapping epochs per subject. They are therefore not - # declared as trial events, but remain available as annotations on - # the loaded raw data (see ``_convert_run_001_2022``). + # Waypoint/end codes (16/48/255) are instantaneous point markers, + # not 90 s trials; see the class docstring. events={"trajectory_start": 1}, code="BNCI2022-001", interval=[0, 90], # Approximately 90 seconds per trajectory diff --git a/moabb/datasets/kaneshiro2015.py b/moabb/datasets/kaneshiro2015.py index 259cea1817..9e6ea2c979 100644 --- a/moabb/datasets/kaneshiro2015.py +++ b/moabb/datasets/kaneshiro2015.py @@ -143,11 +143,7 @@ def __init__(self, subjects=None, sessions=None): events=dict(_EVENTS), code="Kaneshiro2015", interval=[0, 0.496], - # Six object categories with no Target/NonTarget structure, so - # the P300 paradigm cannot process this dataset. The "imagery" - # tag routes it to the n-class paradigms (as done for other - # non-MI multiclass datasets, e.g. BNCI2022_001). - paradigm="imagery", + paradigm="imagery", # 6-class visual ERP, no Target/NonTarget doi=_DOI, selected_subjects=subjects, selected_sessions=sessions, diff --git a/moabb/datasets/lee2024.py b/moabb/datasets/lee2024.py index 0747236fd7..2816eacaff 100644 --- a/moabb/datasets/lee2024.py +++ b/moabb/datasets/lee2024.py @@ -5,6 +5,7 @@ Data: https://github.com/jml226/Home-Appliance-Control-Dataset """ +import json import logging from functools import partialmethod from pathlib import Path @@ -33,6 +34,10 @@ _GITHUB_RAW = ( "https://raw.githubusercontent.com/jml226/Home-Appliance-Control-Dataset/main" ) +_GITHUB_API_TREE = ( + "https://api.github.com/repos/jml226/Home-Appliance-Control-Dataset" + "/git/trees/main?recursive=1" +) _DOI = "10.3389/fnhum.2024.1320457" _SIGN = "lee2024erp" @@ -438,6 +443,45 @@ def _build_raw(signals, trigger, config): return raw + _upstream_paths = None + + @classmethod + def _upstream_tree(cls): + """Blob paths of the upstream repo -- fetched once per process.""" + if cls._upstream_paths is None: + import requests + + resp = requests.get(_GITHUB_API_TREE, timeout=120) + resp.raise_for_status() + cls._upstream_paths = [ + e["path"] for e in resp.json()["tree"] if e["type"] == "blob" + ] + return cls._upstream_paths + + def _subject_files(self, config, subj_str): + """One subject's files, as upstream-relative paths under the experiment. + + The NEMAR deposit's provenance manifest is the preferred inventory -- + it mirrors the upstream names exactly and is already local after the + sourcedata store is fetched, so the NEMAR path never contacts the + upstream host at all. Without a store (provider ``"upstream"``), the + upstream git tree is the authoritative inventory (gh-1142: guessing + filename patterns silently dropped 345 files, e.g. subject 8's + unpadded ``sub8_*`` names and every ``param.mat``). + """ + store = self._sourcedata_store() + manifest = store / "sourcedata_provenance.json" if store else None + if manifest and manifest.is_file(): + rels = (f["file"] for f in json.loads(manifest.read_text())["files"]) + else: + prefix = config["dir_name"] + "/" + rels = ( + path[len(prefix) :] + for path in self._upstream_tree() + if path.startswith(prefix) + ) + return sorted(r for r in rels if r.startswith(f"Dat_{subj_str}/")) + def data_path( self, subject, path=None, force_update=False, update_path=None, verbose=None ): @@ -445,51 +489,21 @@ def data_path( raise ValueError("Invalid subject number") config = _EXPERIMENT_CONFIGS[self._experiment] - subj_dir = self._subject_dir(subject, path) subj_str = self._subj_str(subject, config) - - import requests as _requests - - files_to_dl = [] - - # Testing blocks. - for i in range(1, 31): - files_to_dl.append(f"{subj_str}_Testing{i}.mat") - - # Training blocks. Upstream ships combined and per-block files in - # mixtures the configs do not capture (AirConditioner is "combined" - # yet six subjects also have Training1..50), so request both forms - # and let the 404 skip sort out which exist for this subject (gh-1142). - if config["has_training"]: - files_to_dl.append(f"{subj_str}_Training.mat") - files_to_dl.extend(f"{subj_str}_Training{i}.mat" for i in range(1, 51)) - - # Calibration signal and recording parameters. - files_to_dl += ["cal_sig.mat", "param.mat"] - - subj_dir.mkdir(parents=True, exist_ok=True) - - for fname in files_to_dl: - local = subj_dir / fname - if local.exists() and not force_update: - continue - # Subject 8's files carry the UNPADDED id upstream - # (Dat_sub08/sub8_Testing1.mat), so a 404 under the padded name - # retries unpadded. Any other failure now raises instead of - # leaving a silently incomplete directory (gh-1142). - for name in dict.fromkeys([fname, fname.replace(subj_str, f"sub{subject}")]): - url = f"{_GITHUB_RAW}/{config['dir_name']}/Dat_{subj_str}/{name}" - resp = _requests.get(url, stream=True, timeout=120) - if resp.status_code == 404: - continue - resp.raise_for_status() - log.info("Downloading %s ...", name) - with open(local, "wb") as fout: - for chunk in resp.iter_content(chunk_size=8192): - fout.write(chunk) - break - - return str(subj_dir) + unpadded = self._subj_str(subject, {"zero_pad": False}) + for rel in self._subject_files(config, subj_str): + # The loader constructs zero-padded names; subject 8's upstream + # files are unpadded (sub8_*), so normalize the local name. + local = rel.replace(f"{unpadded}_", f"{subj_str}_") + dl.data_dl( + f"{_GITHUB_RAW}/{config['dir_name']}/{rel}", + _SIGN, + path=path, + force_update=force_update, + verbose=verbose, + fname=f"{config['dir_name']}/{local}", + ) + return str(self._subject_dir(subject, path)) class Lee2024_TV(Lee2024): diff --git a/moabb/paradigms/resting_state.py b/moabb/paradigms/resting_state.py index 1837a462ce..4b69b7e01b 100644 --- a/moabb/paradigms/resting_state.py +++ b/moabb/paradigms/resting_state.py @@ -72,9 +72,7 @@ def __init__( ) def used_events(self, dataset): - if self.events is None: - return dict(dataset.event_id) - return {ev: dataset.event_id[ev] for ev in self.events} + return {ev: dataset.event_id[ev] for ev in self.events or dataset.event_id} def is_valid(self, dataset): ret = True diff --git a/moabb/tests/test_bnci.py b/moabb/tests/test_bnci.py index cc6eac39da..bc0b022b5c 100644 --- a/moabb/tests/test_bnci.py +++ b/moabb/tests/test_bnci.py @@ -123,26 +123,8 @@ def _load_fake_2022_001_raw(tmp_path): return raw, n_traj -def test_bnci2022_001_declares_only_trajectory_trials(): - """Point events must not be declared as 90 s trials. - - The waypoint hit/miss and trajectory-end triggers are instantaneous - markers occurring ~1000 times per subject; combined with the 90 s trial - interval they made default paradigm epoching try to allocate hundreds of - GiB (gh-1143, defect 1). Only the ~90 s trajectory is a trial. - """ - dataset = BNCI2022_001() - assert dataset.event_id == {"trajectory_start": 1} - assert dataset.interval == [0, 90] - - def test_bnci2022_001_trigger_pulses_annotated_once(tmp_path): - """Each held trigger pulse must yield exactly one annotation. - - The 8-bit hardware trigger holds each event code over several consecutive - samples; annotating every non-zero sample multiplied each real event into - dozens of duplicates (33k+ events per subject on develop). - """ + """Each held trigger pulse yields one annotation, not one per sample (gh-1143).""" raw, n_traj = _load_fake_2022_001_raw(tmp_path) desc = raw.annotations.description assert np.sum(desc == "trajectory_start") == n_traj @@ -152,15 +134,10 @@ def test_bnci2022_001_trigger_pulses_annotated_once(tmp_path): def test_bnci2022_001_default_epoching_is_bounded(tmp_path): - """Default trial derivation must stay within the recording's duration. - - Applies the dataset's own event_id/interval the way both - ``BaseDataset.get_data`` and the paradigm pipelines do (via - ``SetRawAnnotations``) and checks that the resulting trials are the - n_trajectories 90 s trajectories, not thousands of overlapping 90 s - epochs anchored on instantaneous waypoint markers. - """ + """Only the ~90 s trajectories are trials; point markers are not (gh-1143).""" dataset = BNCI2022_001() + assert dataset.event_id == {"trajectory_start": 1} + assert dataset.interval == [0, 90] raw, n_traj = _load_fake_2022_001_raw(tmp_path) transform = SetRawAnnotations(dataset.event_id, interval=tuple(dataset.interval)) raw = transform.transform(raw) diff --git a/moabb/tests/test_dataset_fixes.py b/moabb/tests/test_dataset_fixes.py index f01100a2a9..b77fd63404 100644 --- a/moabb/tests/test_dataset_fixes.py +++ b/moabb/tests/test_dataset_fixes.py @@ -305,30 +305,14 @@ def test_schirrmeister2017_reuses_relocated_files(tmp_path: Path, monkeypatch): def test_kaneshiro2015_valid_for_declared_paradigm(): - """Kaneshiro2015 must be accepted by the paradigm it declares. - - The dataset has six object categories and no Target/NonTarget - events, so it cannot satisfy the P300 paradigm; it is tagged - "imagery" to route it to the n-class paradigms (like BNCI2022_001). - """ + """Kaneshiro2015 is accepted by its declared paradigm (gh-1143): six object + categories, no Target/NonTarget, so "imagery" routes it to n-class paradigms.""" from moabb.paradigms import P300, Imagery, MotorImagery dataset = Kaneshiro2015() - assert dataset.paradigm == "imagery" - assert dataset.metadata.experiment.paradigm == dataset.paradigm - for paradigm in (MotorImagery(), Imagery(), MotorImagery(n_classes=6)): assert paradigm.is_valid(dataset) - used = paradigm.used_events(dataset) - assert used == { - "human_body": 1, - "human_face": 2, - "animal_body": 3, - "animal_face": 4, - "fruit_vegetable": 5, - "inanimate_object": 6, - } - + assert paradigm.used_events(dataset) == dataset.event_id # The old declaration was broken: P300 requires Target/NonTarget. assert not P300().is_valid(dataset) diff --git a/moabb/tests/test_datasets.py b/moabb/tests/test_datasets.py index 0808c6664d..10dda9c7d4 100644 --- a/moabb/tests/test_datasets.py +++ b/moabb/tests/test_datasets.py @@ -1,4 +1,5 @@ import inspect +import json import logging import re import warnings @@ -1757,34 +1758,52 @@ def test_constructor_summary_table_cross_ref(dataset_cls): ) -def test_lee2024_data_path_requests_every_upstream_form(tmp_path, monkeypatch): - """gh-1142: padded AND unpadded names, both training forms, param.mat; - a non-404 failure raises instead of leaving a silently incomplete dir.""" - import requests - +def test_lee2024_data_path_downloads_the_real_upstream_inventory(tmp_path, monkeypatch): + """gh-1142: the file list comes from a real inventory (NEMAR manifest or + upstream git tree), subject 8's unpadded upstream names are normalized to + the padded names the loader reads, and every fetch goes through data_dl.""" + import moabb.datasets.download as dl from moabb.datasets import Lee2024_DL + from moabb.datasets.lee2024 import Lee2024 + + inventory = [ + "Doorlock/Dat_sub08/sub8_Testing1.mat", + "Doorlock/Dat_sub08/cal_sig.mat", + "Doorlock/Dat_sub01/sub01_Testing1.mat", + "Doorlock/Dat_sub01/param.mat", + ] + monkeypatch.setattr(Lee2024, "_upstream_paths", inventory) + calls = [] + monkeypatch.setattr( + dl, "data_dl", lambda url, sign, fname=None, **k: calls.append((url, fname)) + ) - seen = [] - - class _NotFound: - status_code = 404 - - monkeypatch.setattr(requests, "get", lambda url, **k: seen.append(url) or _NotFound()) + Lee2024_DL().data_path(8, path=str(tmp_path)) + assert calls == [ + ( + "https://raw.githubusercontent.com/jml226/Home-Appliance-Control-Dataset" + "/main/Doorlock/Dat_sub08/cal_sig.mat", + "Doorlock/Dat_sub08/cal_sig.mat", + ), + ( + "https://raw.githubusercontent.com/jml226/Home-Appliance-Control-Dataset" + "/main/Doorlock/Dat_sub08/sub8_Testing1.mat", + "Doorlock/Dat_sub08/sub08_Testing1.mat", # padded for the loader + ), + ] + + calls.clear() + # With a NEMAR store manifest present, the inventory is read from it and + # the upstream tree is never consulted. + store = tmp_path / "store" + store.mkdir() + (store / "sourcedata_provenance.json").write_text( + json.dumps({"files": [{"file": "Dat_sub01/sub01_Testing1.mat"}]}) + ) dataset = Lee2024_DL() - dataset.data_path(8, path=str(tmp_path)) - urls = "\n".join(seen) - assert "sub08_Testing1.mat" in urls # padded first - assert "sub8_Testing1.mat" in urls # unpadded retry (upstream layout) - assert "sub08_Training.mat" in urls # combined form - assert "sub08_Training50.mat" in urls # per-block form - assert "param.mat" in urls - - class _Boom: - status_code = 500 - - def raise_for_status(self): - raise requests.HTTPError("500") - - monkeypatch.setattr(requests, "get", lambda url, **k: _Boom()) - with pytest.raises(requests.HTTPError): - dataset.data_path(1, path=str(tmp_path)) + monkeypatch.setattr(dataset, "_sourcedata_store", lambda: store) + monkeypatch.setattr( + Lee2024, "_upstream_tree", classmethod(lambda cls: pytest.fail("tree used")) + ) + dataset.data_path(1, path=str(tmp_path)) + assert [fname for _, fname in calls] == ["Doorlock/Dat_sub01/sub01_Testing1.mat"] From caa9205657c04b2e68dea9aa54f57bd1129ed4d4 Mon Sep 17 00:00:00 2001 From: Bruno Aristimunha Date: Fri, 21 Aug 2026 07:43:55 -0700 Subject: [PATCH 7/7] Store lookup: keep URL-tail fallback when fname is given fname names the destination (Lee2024 prefixes the experiment directory and zero-pads subject 8), while the store keeps the upstream names, so probing only fname missed every prefetched file and the NEMAR path fell back to GitHub. Probe fname first, then the URL tails, and cover it with a sockets-forbidden regression test. --- moabb/datasets/download.py | 20 +++++++++++--------- moabb/tests/test_download.py | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/moabb/datasets/download.py b/moabb/datasets/download.py index aa7d092b00..b8fe1ef50e 100644 --- a/moabb/datasets/download.py +++ b/moabb/datasets/download.py @@ -58,19 +58,21 @@ def _store_lookup(url, fname=None): """Serve ``url``'s file from the active local store, if present. The store keeps the upstream relative layout, so the file is probed by the - trailing segments of the URL path, longest tail first; an explicit - ``fname`` is probed as-is. A miss returns None and the caller downloads. + trailing segments of the URL path, longest tail first. An explicit + ``fname`` is probed first, but the URL tails remain fallbacks: a caller's + ``fname`` names the *destination* (e.g. Lee2024 prefixes the experiment + directory and zero-pads subject 8), while the store keeps the upstream + names, so the two can legitimately differ. A miss returns None and the + caller downloads. """ store = _ACTIVE_STORE.get() if store is None: return None - if fname is not None: - tails = [PurePosixPath(fname).parts] - else: - parts = PurePosixPath(urlparse(url).path).parts - # ponytail: three trailing segments cover every current dataset layout; - # deepen if a deposit ever nests further. - tails = [parts[i:] for i in range(max(len(parts) - 3, 0), len(parts))] + tails = [PurePosixPath(fname).parts] if fname is not None else [] + parts = PurePosixPath(urlparse(url).path).parts + # ponytail: three trailing segments cover every current dataset layout; + # deepen if a deposit ever nests further. + tails += [parts[i:] for i in range(max(len(parts) - 3, 0), len(parts))] for tail in tails: if not tail: continue diff --git a/moabb/tests/test_download.py b/moabb/tests/test_download.py index 7a112fb695..618a0c06ab 100644 --- a/moabb/tests/test_download.py +++ b/moabb/tests/test_download.py @@ -1189,3 +1189,27 @@ def test_get_data_prefetches_the_requested_subjects(monkeypatch): assert fetched == [1, 3] assert sorted(data) == [1, 3] + + +def test_store_lookup_falls_back_to_url_tails_when_fname_differs(tmp_path, monkeypatch): + """gh-1151 review: fname names the destination (experiment prefix, padded + subject), the store keeps upstream names -- the URL tails must still hit.""" + store = tmp_path / "store" + (store / "Dat_sub08").mkdir(parents=True) + (store / "Dat_sub08" / "sub8_Testing1.mat").write_text("mirrored") + monkeypatch.setattr(socket.socket, "connect", _forbid_connect) + + url = ( + "https://raw.githubusercontent.com/jml226/Home-Appliance-Control-Dataset" + "/main/Doorlock/Dat_sub08/sub8_Testing1.mat" + ) + with dl.active_sourcedata_store(store): + path = dl.data_dl( + url, + "lee2024erp", + path=tmp_path, + fname="Doorlock/Dat_sub08/sub08_Testing1.mat", + ) + + assert Path(path).read_text() == "mirrored" + assert Path(path).name == "sub08_Testing1.mat" # destination keeps fname