diff --git a/docs/source/whats_new.rst b/docs/source/whats_new.rst index bdb3141a2..6072b1245 100644 --- a/docs/source/whats_new.rst +++ b/docs/source/whats_new.rst @@ -63,6 +63,42 @@ Requirements Bugs ~~~~ +- Fix :class:`moabb.datasets.Lenaig2026`'s ``data_path()`` raising + ``FileNotFoundError: Some data files are missing.`` on every subject: it + hard-coded an ``EEG_24Chan_AudioStim/`` wrapper directory for the extracted + RAR, but the current Zenodo v2 archive (record ``21156618``) extracts + ``EXP1/``/``EXP2/`` directly at the root -- confirmed by direct inspection + on Voyager (``find ... -iname '*EEG_24Chan*'`` only finds the ``.rar`` + itself). ``data_path()`` now locates each run's file with a recursive glob + that matches either layout, without changing which events/labels are read + (:gh:`1225` by `Bruno Aristimunha`_). +- Fix :class:`moabb.datasets.Schrag2026Pediatric` crashing + ``convert_to_bids()`` with ``ValueError: Raw object must have annotations + to be saved in BIDS format`` on subject 1's personalized-stimulus game run: + the loader's own documented policy of dropping all trial labels when a + run's ``Trial Started`` marker count drifts more than 10%% from its + movements-CSV row count (true for that run, 15%% drift) produced a + zero-annotation ``Raw``, which ``bids_interface``'s writer then rejected. + ``_get_single_subject_data`` now skips a run that ends up with zero + events instead of returning it, logging a warning that names the subject + and run; the docstring documents this (:gh:`1225` by `Bruno Aristimunha`_). +- Fix :class:`moabb.datasets.Lenaig2026` crashing ``convert_to_bids()`` with + ``AttributeError: 'int' object has no attribute 'items'``: + ``METADATA.experiment.trials_per_class`` was a bare int (``10``) instead + of the ``Dict[str, int]`` the schema declares, and + ``bids_interface._build_readme`` unconditionally calls + ``_format_dict()`` on it. Now a per-class dict (``{"Stimulus": 10, + "Silence": 10}``), matching the documented 10 repetitions per condition + (:gh:`1225` by `Bruno Aristimunha`_). +- Fix :func:`moabb.datasets.Dataset.convert_to_bids` aborting the whole + multi-subject convert when exactly one subject raises + ``FileNotFoundError`` (a loader's own "nothing to write for this subject" + signal, e.g. :class:`moabb.datasets.Schrag2026Pediatric` subject 16's + single game run being dropped entirely by the >10%% drift policy): every + subject after the one that raised was previously silently skipped as + well. The per-subject loop now catches ``FileNotFoundError``, logs a + warning naming the subject, and continues converting the rest + (:gh:`1225` by `Bruno Aristimunha`_). - Keep :class:`moabb.evaluations.CrossSubjectEvaluation` result provenance subject-specific when a grouped cross-validation fold holds out multiple subjects at once. The estimator is still fitted once per fold, while scores, cache identities, and saved-model paths are emitted per held-out subject and session instead of assigning the whole fold to its first subject (by `lindicaphxag-tech`_). - Fix :func:`moabb.datasets.Dataset.convert_to_bids` crashing on datasets whose MOABB run-label suffix is literally ``"calibration"`` or ``"crosstalk"`` diff --git a/moabb/datasets/base.py b/moabb/datasets/base.py index f3608ab69..1a0dedff6 100644 --- a/moabb/datasets/base.py +++ b/moabb/datasets/base.py @@ -1400,7 +1400,17 @@ def convert_to_bids( repr(interface), ) continue - sessions_data = self.get_data(subjects=[subject]) + try: + sessions_data = self.get_data(subjects=[subject]) + except FileNotFoundError as exc: + # A loader's own "no usable data for this subject" signal + # (e.g. every run dropped by a documented quality policy, + # not a download/path bug) must not abort every other + # subject's conversion in the same call. Skip loudly instead. + log.warning( + "%s: skipping subject %s, no usable data: %s", self.code, subject, exc + ) + continue interface.save(sessions_data[subject]) bids_root = get_bids_root(self.code, path) diff --git a/moabb/datasets/lenaig2026.py b/moabb/datasets/lenaig2026.py index 684cd00eb..5d4814810 100644 --- a/moabb/datasets/lenaig2026.py +++ b/moabb/datasets/lenaig2026.py @@ -171,7 +171,14 @@ class Lenaig2026(BaseDataset): paradigm="ssvep", n_classes=2, class_labels=["Stimulus", "Silence"], - trials_per_class=10, + # 10 repetitions per condition (5 conditions: Sinus, BrownNoise, + # Cicada, Cat, Silence) per the dataset description above; the + # schema declares ``trials_per_class`` as Dict[str, int], not a + # bare int -- bids_interface._build_readme's + # ``_format_dict(exp.trials_per_class)`` crashes with + # ``AttributeError: 'int' object has no attribute 'items'`` + # otherwise (hit on a real end-to-end convert). + trials_per_class={"Stimulus": 10, "Silence": 10}, trial_duration=10, feedback_type="auditory", mode="offline", @@ -292,27 +299,50 @@ def data_path( if path else Path(dl.get_dataset_path(_SIGN, None)) / f"MNE-{_SIGN}-data" ) - paths = [ - base_path - / "EEG_24Chan_AudioStim" - / f"EXP{self.exp}/S1/R{run}/{subject:02}_R{run}.gdf" - for run in self.runs + rel_patterns = [ + f"EXP{self.exp}/S1/R{run}/{subject:02}_R{run}.gdf" for run in self.runs ] - if all(p.exists() for p in paths) and not force_update: - return ( - paths # Return existing paths if all files exist and no update is forced - ) + if not force_update: + paths = self._locate_files(base_path, rel_patterns) + if paths is not None: + return paths url = f"{_ZENODO_BASE}/EEG_24Chan_AudioStim.rar" rar_path = Path(dl.data_dl(url, sign=_SIGN, path=base_path, verbose=verbose)) extract_rar(rar_path, dest_dir=base_path) - if not all(p.exists() for p in paths) or force_update: + paths = self._locate_files(base_path, rel_patterns) + if paths is None: raise FileNotFoundError("Some data files are missing.") return paths + @staticmethod + def _locate_files(base_path, rel_patterns): + """Locate each relative file pattern under ``base_path``. + + The Zenodo record's RAR archive has shipped at least two extracted + layouts over time: wrapped under an ``EEG_24Chan_AudioStim/`` + directory (what this loader originally assumed) and flat, with the + ``EXP*/`` directories directly at the extraction root (the current + Zenodo v2 archive, record ``21156618``). ``Path.glob("**/")`` + matches both -- the recursive ``**`` segment matches zero or more + intermediate directories -- without hard-coding either layout. Does + not change which events/trials are read, only how the source files + are found. + + Returns the list of matched paths (one per pattern, in order) or + ``None`` if any pattern has no match. + """ + found = [] + for rel in rel_patterns: + matches = sorted(base_path.glob(f"**/{rel}")) + if not matches: + return None + found.append(matches[0]) + return found + def _get_single_subject_data(self, subject): """ Load and preprocess raw EEG data for a single subject. diff --git a/moabb/datasets/schrag2026.py b/moabb/datasets/schrag2026.py index fcc9a705f..fc373005d 100644 --- a/moabb/datasets/schrag2026.py +++ b/moabb/datasets/schrag2026.py @@ -126,6 +126,18 @@ class Schrag2026Pediatric(BaseDataset): corner-to-frequency mapping (randomised across the game; not currently exposed by this loader). + .. warning:: + A game run whose ``Trial Started`` marker count drifts more than + 10%% from its movements-CSV row count has all its labels dropped + (see :func:`_load_game_run`); that run is then **skipped** rather + than returned, with a logged warning naming the subject and run. + At least one subject (subject 1, personal-stimulus run) is known + to hit this on the released archive. A subject whose every game + run drifts this way yields zero runs; ``_get_single_subject_data`` + then raises ``FileNotFoundError`` instead of writing an + unannotated ``Raw`` (BIDS requires every ``Raw`` to carry + annotations). + .. note:: Zenodo publishes one archive per subject, so loading a subject downloads only that subject (~20-40 MB). @@ -295,7 +307,23 @@ def _get_single_subject_data(self, subject): if m is None: continue key = _R_STD if m.group(2) == "BW" else _R_PERS - runs[key] = _load_game_run(path) + raw = _load_game_run(path) + if len(raw.annotations) == 0: + # The >10% Trial/CSV drift policy in _load_game_run already + # dropped every label for this run (logged there as an + # [ERROR]); a zero-annotation Raw cannot be written to BIDS + # (bids_interface._write_file requires annotations), so skip + # the run entirely rather than return unusable data. + log.warning( + "Subject %d: run %r (%s) has zero labelled trials after " + "the >10%% drift policy dropped all labels; skipping " + "this run instead of returning an unannotated Raw.", + subject, + key, + path.name, + ) + continue + runs[key] = raw # Personalization (T1) is a single XDF per subject, opt-in, third run. if self.include_personalization: @@ -424,7 +452,9 @@ def _load_game_run(fpath): Trials are paired with CSV rows by index. Some sessions have a few extra trailing CSV rows from end-of-game bookkeeping; if the count drift is large (>10 percent) we drop the run's labels entirely rather - than emit silently-shifted ones. + than emit silently-shifted ones. The caller (``_get_single_subject_data``) + then skips the resulting zero-annotation run instead of returning it, so + a high-drift run never reaches the BIDS writer unannotated. """ eeg_stream, marker_stream = _load_xdf_streams(fpath) marker_ts, markers = _read_unity_markers(marker_stream) diff --git a/moabb/tests/test_dataset_fixes.py b/moabb/tests/test_dataset_fixes.py index b77fd6340..52eb81373 100644 --- a/moabb/tests/test_dataset_fixes.py +++ b/moabb/tests/test_dataset_fixes.py @@ -13,9 +13,11 @@ from moabb.datasets import schirrmeister2017 from moabb.datasets.bnci.bnci_2020 import _convert_attention_shift from moabb.datasets.braininvaders import BI2015b +from moabb.datasets.fake import FakeDataset from moabb.datasets.hefmi_ich2025 import HefmiIch2025 from moabb.datasets.kaneshiro2015 import Kaneshiro2015 from moabb.datasets.kojima2024a import Kojima2024A +from moabb.datasets.lenaig2026 import Lenaig2026 from moabb.datasets.mainsah2025 import _parse_manifest from moabb.datasets.schirrmeister2017 import Schirrmeister2017 from moabb.datasets.ssvep_chen2017 import Chen2017SingleFlicker @@ -316,3 +318,136 @@ def test_kaneshiro2015_valid_for_declared_paradigm(): assert paradigm.used_events(dataset) == dataset.event_id # The old declaration was broken: P300 requires Target/NonTarget. assert not P300().is_valid(dataset) + + +def test_lenaig2026_data_path_accepts_wrapped_and_flat_layouts(tmp_path, monkeypatch): + """data_path() assumed the extracted RAR always sits under an + ``EEG_24Chan_AudioStim/`` wrapper directory, but the current Zenodo v2 + archive (record 21156618) extracts ``EXP1/``/``EXP2/`` directly at the + root -- confirmed by direct inspection on Voyager. The loader must find + the files either way, without touching events/labels.""" + import moabb.datasets.lenaig2026 as lenaig2026_mod + + def make_tree(root): + for run in (1, 2): + run_dir = root / "EXP1" / "S1" / f"R{run}" + run_dir.mkdir(parents=True) + (run_dir / f"01_R{run}.gdf").write_bytes(b"") + + # Flat layout: EXP1/ directly at the extraction root (current archive). + flat_root = tmp_path / "flat" / "MNE-Lenaig2026-data" + make_tree(flat_root) + monkeypatch.setattr( + lenaig2026_mod.dl, "get_dataset_path", lambda *a, **k: str(tmp_path / "flat") + ) + paths = Lenaig2026(exp=1, run="both").data_path(1) + assert [Path(p).name for p in paths] == ["01_R1.gdf", "01_R2.gdf"] + assert all(Path(p).is_file() for p in paths) + + # Wrapped layout: EXP1/ under EEG_24Chan_AudioStim/ (what the loader + # originally -- and exclusively -- assumed). + wrapped_root = tmp_path / "wrapped" / "MNE-Lenaig2026-data" + make_tree(wrapped_root / "EEG_24Chan_AudioStim") + monkeypatch.setattr( + lenaig2026_mod.dl, "get_dataset_path", lambda *a, **k: str(tmp_path / "wrapped") + ) + paths2 = Lenaig2026(exp=1, run="both").data_path(1) + assert [Path(p).name for p in paths2] == ["01_R1.gdf", "01_R2.gdf"] + assert all(Path(p).is_file() for p in paths2) + + +def test_convert_to_bids_skips_subject_with_no_usable_data(tmp_path, monkeypatch, caplog): + """gh: Schrag2026Pediatric subject 16 has a single game recording whose + only run is dropped by the >10%% drift policy, leaving zero usable runs; + _get_single_subject_data correctly raises FileNotFoundError for that one + subject (a loader's own "nothing to write" signal). Before this fix, + convert_to_bids's per-subject loop had no try/except around + self.get_data(), so that single subject's FileNotFoundError aborted the + whole multi-subject convert, losing every subject not yet processed. + convert_to_bids must now skip that subject (with a warning) and keep + converting the rest.""" + from moabb.datasets.base import BaseDataset + + dataset = FakeDataset(event_list=["fake1", "fake2"], n_sessions=1, n_subjects=3) + real_get_data = BaseDataset.get_data + + def flaky_get_data(self, subjects=None, **kwargs): + if subjects == [2]: + raise FileNotFoundError("subject 2: no usable runs after drift policy") + return real_get_data(self, subjects=subjects, **kwargs) + + monkeypatch.setattr(FakeDataset, "get_data", flaky_get_data) + + with caplog.at_level("WARNING"): + bids_root = dataset.convert_to_bids(path=tmp_path, subjects=[1, 2, 3]) + + assert any("skipping subject" in record.message for record in caplog.records), ( + "a warning naming the skipped subject must be logged" + ) + subjects_found = {f.parent.parent.parent.name for f in bids_root.rglob("*.edf")} + assert subjects_found == {"sub-1", "sub-3"}, "subject 2 must be skipped, not abort" + + +def test_lenaig2026_trials_per_class_builds_a_readme(tmp_path): + """gh: METADATA.experiment.trials_per_class was a bare int (``10``), + violating the schema's declared ``Dict[str, int]`` type. A real + end-to-end convert crashed in ``bids_interface._build_readme`` with + ``AttributeError: 'int' object has no attribute 'items'`` because + ``_format_dict`` assumes a mapping. It is now a per-class dict; the + README builder must run without crashing.""" + from moabb.datasets.bids_interface import _build_readme + + dataset = Lenaig2026(exp=1, run="both") + assert isinstance(dataset.METADATA.experiment.trials_per_class, dict) + readme = _build_readme(dataset) + assert "Trials per class" in readme + + +def test_schrag2026_skips_run_with_zero_annotations_after_high_drift( + tmp_path, monkeypatch, caplog +): + """gh: subject 1's personal-stimulus game run has >10%% Trial/CSV drift, + so ``_load_game_run``'s documented policy drops every label, returning a + zero-annotation Raw. ``bids_interface._write_file`` hard-requires every + Raw to carry annotations, so writing that Raw crashes the whole convert. + ``_get_single_subject_data`` must skip (not return) a zero-annotation + run, with a warning naming the subject and run, instead of crashing + downstream.""" + import mne + import numpy as np + + from moabb.datasets import schrag2026 + from moabb.datasets.schrag2026 import Schrag2026Pediatric + + eeg_dir = tmp_path / "P001" / "EEG" + eeg_dir.mkdir(parents=True) + std_name = "sub-P001_ses-S001_task-T2_acq-BW_M1_run-001_eeg.xdf" + pers_name = "sub-P001_ses-S001_task-T3_acq-C3S1_M2_run-001_eeg.xdf" + (eeg_dir / std_name).write_bytes(b"") + (eeg_dir / pers_name).write_bytes(b"") + + info = mne.create_info(["Fz"], 256.0, "eeg") + + def fake_load_game_run(path): + raw = mne.io.RawArray(np.zeros((1, 10)), info, verbose=False) + if path.name == pers_name: + return raw # the >10%% drift run: zero annotations + raw.set_annotations( + mne.Annotations(onset=[0.0], duration=[5.0], description=["6.25"]) + ) + return raw + + monkeypatch.setattr(schrag2026, "_load_game_run", fake_load_game_run) + dataset = Schrag2026Pediatric() + monkeypatch.setattr(dataset, "data_path", lambda *a, **k: str(tmp_path / "P001")) + + with caplog.at_level("WARNING"): + session = dataset._get_single_subject_data(1) + + runs = session["0"] + assert set(runs) == {"0standard"}, "the zero-annotation run must be skipped" + assert len(runs["0standard"].annotations) == 1 + assert any( + "zero labelled trials" in record.message and "1personal" in record.message + for record in caplog.records + ), "a warning naming the subject/run must be logged"