diff --git a/.github/workflows/download-test.yml b/.github/workflows/download-test.yml index 38cfe2918..bc9925238 100644 --- a/.github/workflows/download-test.yml +++ b/.github/workflows/download-test.yml @@ -38,5 +38,4 @@ jobs: - name: Run download tests run: | echo "Running tests" - cd moabb/tests - pytest -vv -s --tb=long --durations=0 --maxfail=5 --log-cli-level=INFO --dl-data download.py + pytest -vv -s --tb=long --durations=0 --maxfail=5 --log-cli-level=INFO --dl-data moabb/tests/test_download.py diff --git a/docs/source/whats_new.rst b/docs/source/whats_new.rst index b1996c668..5e1314cd4 100644 --- a/docs/source/whats_new.rst +++ b/docs/source/whats_new.rst @@ -18,8 +18,8 @@ What's new .. _current: -Version 1.7 (Source - GitHub) -------------------------------- +Version 1.6.1 (Source - GitHub) +--------------------------------- Enhancements ~~~~~~~~~~~~ @@ -35,7 +35,13 @@ Requirements Bugs ~~~~ -- None yet. +- Point the monthly download job at ``moabb/tests/test_download.py``; it ran ``download.py``, a file that does not exist, so it collected nothing and has been failing every month while no ``@pytest.mark.download`` test executed anywhere in CI (by `Bruno Aristimunha`_). +- Stop importing moabb from restyling the caller's matplotlib. ``moabb.analysis.plotting`` applies a seaborn theme to the global ``rcParams`` when it is imported, and ``moabb/analysis/__init__.py`` imported it eagerly -- so ``import moabb.datasets`` alone changed ``font.family``, ``axes.grid`` and ``axes.spines.*`` for every figure the caller drew afterwards, values matplotlib reads at axes creation and a caller therefore cannot undo. The path was indirect: :mod:`moabb.datasets.bids_interface` imports ``moabb.analysis.results`` for ``get_digest``, which runs that ``__init__``. The plotting imports are now deferred to the functions that use them and to a module-level ``__getattr__`` for the three re-exported plotting helpers, so every public name still resolves and an explicit ``import moabb.analysis.plotting`` still applies the theme, leaving MOABB's own figures unchanged (by `Bruno Aristimunha`_). +- Type the seven non-EEG channels of :class:`moabb.datasets.BNCI2025_001` as ``misc``. ``read_raw_eeglab`` types anything it does not recognise as ``eeg``, so ``x``/``y``/``vx``/``vy``/``validity``/``targetPosX``/``targetPoxY`` -- the hand kinematics and the target position of the reaching task -- were picked as EEG by every paradigm and fed to classifiers as features, leaking the labels they encode. The declared ``n_channels`` (67) also disagreed with the dataset's own 60-electrode montage; it is now 71 with ``channel_types={"eeg": 60, "eog": 4, "misc": 7}``. Accuracies on this dataset will fall, which is the point (by `Bruno Aristimunha`_). +- Give :class:`moabb.datasets.Rodrigues2017` the montage :gh:`700` announced but never shipped, the same omission as :class:`moabb.datasets.Cattan2019_PHMD` below: both share the 16-electrode setup, both spelled ``Fc5``/``Fc6`` -- the only two names ``standard_1020`` cannot resolve -- and neither loader called ``set_montage``. Its ``METADATA`` also declared ``standard_1010``, which is not a montage MNE can build (by `Bruno Aristimunha`_). +- Give :class:`moabb.datasets.Cattan2019_PHMD` the montage :gh:`700` announced but never shipped: that PR fixed only the unit scaling, leaving the loader with no ``set_montage`` call at all. Its channel list also spelled the two frontal-central electrodes ``Fc5``/``Fc6``, the only two of its sixteen names that ``standard_1020`` cannot resolve; they are now ``FC5``/``FC6`` (by `Bruno Aristimunha`_). +- Correct :class:`moabb.datasets.Cattan2019_PHMD` ``interval`` from ``[0, 1]`` to ``[0, 60]``. Each marker starts a one-minute relaxation block -- as the dataset's own ``block_duration_s=60.0`` records -- but ``SetRawAnnotations`` derives annotation durations from ``interval``, so every block was annotated as lasting one second. ``interval[0]`` is unchanged, so onsets do not move (by `Bruno Aristimunha`_). +- Prefetch the NEMAR sourcedata store per subject rather than per dataset. The guard returned as soon as the store held anything, and the deposit's provenance manifest lands inside it, so a store holding only a manifest counted as complete. Presence is now settled per subject from the cached manifest, at no network cost: :func:`nemar.download` walks index, version, metadata and manifest before it consults ``trust_existing``, so a call with nothing to do is still four round-trips. Note that deposits whose manifest predates the ``subject`` field -- which is every deposit today -- are fetched as a whole tree, so for those the older whole-store rule was already right and is kept; the per-subject check matters once manifests record subjects, and today for a store left partial by an interrupted fetch (by `Bruno Aristimunha`_). Code health ~~~~~~~~~~~ diff --git a/moabb/__init__.py b/moabb/__init__.py index 42ad7b4bb..44a125c13 100644 --- a/moabb/__init__.py +++ b/moabb/__init__.py @@ -1,5 +1,5 @@ # flake8: noqa -__version__ = "1.7.0dev0" +__version__ = "1.6.1dev0" from .benchmark import benchmark from .utils import ( diff --git a/moabb/analysis/__init__.py b/moabb/analysis/__init__.py index 7b3b603fa..71835576e 100644 --- a/moabb/analysis/__init__.py +++ b/moabb/analysis/__init__.py @@ -1,3 +1,4 @@ +import importlib import logging import os import platform @@ -5,7 +6,6 @@ from mne.utils import _open_lock -from moabb.analysis import plotting as plt from moabb.analysis.chance_level import ( # noqa: F401 adjusted_chance_level, chance_by_chance, @@ -14,11 +14,6 @@ compute_dataset_statistics, find_significant_differences, ) -from moabb.analysis.plotting import ( # noqa: F401 - codecarbon_plot, - distribution_plot, - emissions_summary, -) from moabb.analysis.results import Results # noqa: F401 from moabb.analysis.style import MOABB_PALETTE, apply_moabb_style # noqa: F401 from moabb.analysis.timeline import ( # noqa: F401 @@ -73,6 +68,8 @@ def analyze(results, out_path, name="analysis", plot=False): else: analysis_path = os.path.join(out_path, name) + from moabb.analysis import plotting as plt + unique_ids = [plt._simplify_names(x) for x in results.pipeline.unique()] simplify = True if len(unique_ids) != len(set(unique_ids)): @@ -97,3 +94,27 @@ def analyze(results, out_path, name="analysis", plot=False): fig.savefig(os.path.join(analysis_path, "scores.pdf")) fig = plt.summary_plot(P, T, simplify=simplify) fig.savefig(os.path.join(analysis_path, "ordering.pdf")) + + +# ``plotting`` applies a seaborn theme to the global matplotlib rcParams at +# import time. Re-exporting its names lazily keeps that off the path of anyone +# who reaches this package for something else -- ``moabb.datasets`` imports +# ``moabb.analysis.results`` for ``get_digest``, so an eager import here +# restyled the figures of every caller who merely imported a dataset. +_PLOTTING_EXPORTS = ("codecarbon_plot", "distribution_plot", "emissions_summary") + + +def __getattr__(name): + # ``plotting`` itself is included: the eager import used to bind it as an + # attribute of this package, so ``moabb.analysis.plotting`` resolved without + # the caller importing the submodule. Dropping that would be a break. + if name == "plotting" or name in _PLOTTING_EXPORTS: + # import_module, not ``from . import plotting``: the latter falls back to + # getattr on this package, which re-enters __getattr__ forever. + plotting = importlib.import_module("moabb.analysis.plotting") + return plotting if name == "plotting" else getattr(plotting, name) + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + + +def __dir__(): + return sorted(set(globals()) | set(_PLOTTING_EXPORTS) | {"plotting"}) diff --git a/moabb/datasets/alphawaves.py b/moabb/datasets/alphawaves.py index db6474d16..a958d1b90 100644 --- a/moabb/datasets/alphawaves.py +++ b/moabb/datasets/alphawaves.py @@ -91,7 +91,7 @@ class Rodrigues2017(BaseDataset): sampling_rate=512.0, n_channels=16, channel_types={"eeg": 16}, - montage="standard_1010", + montage="standard_1020", hardware="g.tec g.USBamp", sensor_type="wet electrodes", reference="right earlobe", @@ -99,8 +99,8 @@ class Rodrigues2017(BaseDataset): filters="no digital filter", sensors=[ "Cz", - "Fc5", - "Fc6", + "FC5", + "FC6", "Fp1", "Fp2", "Fz", @@ -206,9 +206,9 @@ def _get_single_subject_data(self, subject): chnames = [ "Fp1", "Fp2", - "Fc5", + "FC5", "Fz", - "Fc6", + "FC6", "T7", "Cz", "T8", @@ -229,6 +229,7 @@ def _get_single_subject_data(self, subject): ch_names=chnames, sfreq=512, ch_types=chtypes, verbose=False ) raw = mne.io.RawArray(data=X, info=info, verbose=False) + raw.set_montage("standard_1020") return {"0": {"0": raw}} diff --git a/moabb/datasets/base.py b/moabb/datasets/base.py index 092024c9e..1721fc01d 100644 --- a/moabb/datasets/base.py +++ b/moabb/datasets/base.py @@ -34,6 +34,7 @@ active_sourcedata_store, nemar_dl, nemar_sourcedata_dl, + nemar_sourcedata_is_local, nemar_store, ) from moabb.datasets.preprocessing import FixedPipeline, SetRawAnnotations @@ -1106,17 +1107,19 @@ def _prefetch_nemar_sourcedata(self, subjects, verbose=None): ``"upstream"`` skips NEMAR entirely, ``"nemar"`` treats a failure as fatal rather than silently reaching the host the caller opted out of, and ``"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 at all -- refresh or extend it with - :meth:`download` (``force_update=True`` to refetch). + dataset's own downloader. Subjects already in the store cost no + network at all: their presence is settled from the cached manifest, + without contacting NEMAR. Refresh one with :meth:`download` + (``force_update=True``). """ provider = get_download_provider() if self.nemar_id is None or provider == "upstream": return - store = self._sourcedata_store() - if store is not None and store.is_dir() and any(store.iterdir()): - return + # Resolved once: nemar_store() re-reads MNE's config file each call. + store_root = nemar_store(self.code, self.nemar_id) for subject in subjects: + if nemar_sourcedata_is_local(store_root, self._nemar_subject_ids(subject)): + continue try: self.sourcedata_path(subject=subject, verbose=verbose) except NemarDownloadError as exc: @@ -1130,6 +1133,22 @@ def _prefetch_nemar_sourcedata(self, subjects, verbose=None): stacklevel=2, ) + def _nemar_subject_ids(self, subject): + """The identifiers a deposit may file this subject under. + + Provenance manifests observed in the wild record raw MOABB ids, but a + dataset's ``nemar_subject_template`` documents how its deposit labels + subjects -- match both, exactly as :meth:`sourcedata_path` does. + """ + label = self._nemar_subject(subject) + # Return the scalar untouched unless the deposit really files this + # subject under a second label: a None label would stringify to "None" + # and collide with manifest entries that carry no subject at all, and + # wrapping a lone id in a list would change what downstream callers see. + if label is None or str(label) == str(subject): + return subject + return [subject, label] + def _sourcedata_store(self): """Local NEMAR sourcedata store to serve this dataset's loads from. @@ -1184,9 +1203,7 @@ def sourcedata_path(self, subject=None, path=None, force_update=False, verbose=N # a dataset's nemar_subject_template documents how its deposit labels # subjects -- match both rather than betting on one convention. if subject is not None: - label = self._nemar_subject(subject) - if label is not None and str(label) != str(subject): - subject = [subject, label] + subject = self._nemar_subject_ids(subject) return nemar_sourcedata_dl( self.nemar_id, self.code, diff --git a/moabb/datasets/bnci/bnci_2025.py b/moabb/datasets/bnci/bnci_2025.py index 4f8f02788..666b2dc88 100644 --- a/moabb/datasets/bnci/bnci_2025.py +++ b/moabb/datasets/bnci/bnci_2025.py @@ -45,6 +45,25 @@ # Base URL for the BNCI 2025-001 dataset (hosted at TU Graz) BNCI_2025_001_URL = "https://lampx.tugraz.at/~bci/database/001-2025/" +# Channels the EEGLAB ``.set`` files carry alongside the EEG montage: hand +# kinematics from the reaching task, the target position, and a validity flag. +# ``read_raw_eeglab`` types anything it cannot recognise as ``eeg``, which would +# feed the target position and hand velocity -- i.e. the labels -- to any +# paradigm picking ``eeg``. +_EOG_CHANNELS_001_2025 = ("EOGL1", "EOGL2", "EOGL3", "EOGR1") + +_NON_EEG_CHANNELS_001_2025 = ( + "x", + "y", + "vx", + "vy", + "validity", + "targetPosX", + "targetPoxY", # sic: the published files misspell targetPosY + "targetPosY", # ...accept the corrected spelling too +) + + # Event code mapping for 001-2025 dataset # Format: XYZ where X=speed (1=slow, 2=fast), Y=distance (1=near, 2=far), Z=direction (1-4) # Direction codes: 1=up, 2=down, 3=left, 4=right @@ -156,7 +175,18 @@ def _load_data_001_2025( return [str(set_file)] # Load the EEGLAB file - raw = mne.io.read_raw_eeglab(str(set_file), preload=True, verbose=verbose) + raw = mne.io.read_raw_eeglab( + str(set_file), eog=_EOG_CHANNELS_001_2025, preload=True, verbose=verbose + ) + + # Type the non-EEG channels before the montage, so they are neither placed + # on the scalp nor picked as EEG. + # set_channel_types raises on a name that is not in info, so filter first. + raw.set_channel_types( + {ch: "misc" for ch in _NON_EEG_CHANNELS_001_2025 if ch in raw.ch_names}, + on_unit_change="ignore", + verbose=False, + ) # Remap annotation descriptions from numeric codes to descriptive names # The data contains codes like "111", "112", etc. which we map to @@ -266,9 +296,9 @@ class BNCI2025_001(BNCIBaseDataset): METADATA = DatasetMetadata( acquisition=AcquisitionMetadata( sampling_rate=500.0, - n_channels=67, - channel_types={"eeg": 67, "eog": 4}, - montage="af7 af3 afz af4 af8 f7 f5 f3 f1 fz f2 f4 f6 f8 ft7 fc5 fc3 fc1 fcz fc2 fc4 fc6 ft8 t7 c5 c3 c1 cz c2 c4 c6 t8 tp7 cp5 cp3 cp1 cpz cp2 cp4 cp6 tp8 p7 p5 p3 p1 pz p2 p4 p6 p8 ppo1h ppo2h po7 po3 poz po4 po8 o1 oz o2", + n_channels=71, + channel_types={"eeg": 60, "eog": 4, "misc": 7}, + montage="standard_1005", sensor_type="EEG", hardware="BrainAmp", reference="common average", @@ -1047,7 +1077,7 @@ class BNCI2025_002(BNCIBaseDataset): sampling_rate=200.0, n_channels=60, channel_types={"eeg": 60, "eog": 4}, - montage="af7 af3 afz af4 af8 f7 f5 f3 f1 fz f2 f4 f6 f8 ft7 fc5 fc3 fc1 fcz fc2 fc4 fc6 ft8 t7 c5 c3 c1 cz c2 c4 c6 t8 tp7 cp5 cp3 cp1 cpz cp2 cp4 cp6 tp8 p7 p5 p3 p1 pz p2 p4 p6 p8 ppo1h ppo2h po7 po3 poz po4 po8 o1 oz o2", + montage="standard_1005", sensor_type="EEG", hardware="actiCAP, Brain Products GmbH", reference="right mastoid", diff --git a/moabb/datasets/braininvaders.py b/moabb/datasets/braininvaders.py index e748abef9..d55645ae3 100644 --- a/moabb/datasets/braininvaders.py +++ b/moabb/datasets/braininvaders.py @@ -182,9 +182,9 @@ def _bi_get_subject_data(ds, subject): # noqa: C901 chnames = [ "Fp1", "Fp2", - "Fc5", + "FC5", "Fz", - "Fc6", + "FC6", "T7", "Cz", "T8", @@ -209,10 +209,13 @@ def _bi_get_subject_data(ds, subject): # noqa: C901 info = mne.create_info( ch_names=chnames, sfreq=sfreq, ch_types=chtypes, verbose=False ) + # Set it here rather than on each Raw: the Cattan2019-VR branch below + # builds many Raws from this Info, and used to leave them with no + # channel positions at all. + info.set_montage(make_standard_montage("standard_1020"), on_missing="ignore") if not ds.code == "Cattan2019-VR": raw = mne.io.RawArray(data=X, info=info, verbose=False) - raw.set_montage(make_standard_montage("standard_1020")) if ds.code == "BrainInvaders2012": # get rid of the Fz channel (it is the ground) @@ -941,7 +944,7 @@ class BI2014a(BaseDataset): sampling_rate=512.0, n_channels=16, channel_types={"eeg": 16}, - montage="standard_1010", + montage="standard_1020", hardware="g.USBamp (g.tec, Schiedlberg, Austria)", sensor_type="dry electrodes", reference="right earlobe", @@ -1143,7 +1146,7 @@ class BI2014b(BaseDataset): sampling_rate=512.0, n_channels=32, channel_types={"eeg": 32}, - montage="standard_1010", + montage="standard_1020", hardware="g.USBamp (g.tec, Schiedlberg, Austria)", sensor_type="wet electrodes", reference="right earlobe", @@ -1356,7 +1359,7 @@ class BI2015a(BaseDataset): sampling_rate=512.0, n_channels=32, channel_types={"eeg": 32}, - montage="10-10", + montage="standard_1020", hardware="g.USBamp (g.tec, Schiedlberg, Austria)", sensor_type="wet electrodes", reference="right earlobe", @@ -1557,7 +1560,7 @@ class BI2015b(BaseDataset): sampling_rate=512.0, n_channels=32, channel_types={"eeg": 32}, - montage="10-10", + montage="standard_1020", hardware="g.USBamp (g.tec, Schiedlberg, Austria)", sensor_type="wet Silver/Silver Chloride electrodes", reference="right earlobe", @@ -1769,7 +1772,7 @@ class Cattan2019_VR(BaseDataset): sampling_rate=512.0, n_channels=16, channel_types={"eeg": 16}, - montage="10-10", + montage="standard_1020", hardware="g.USBamp (g.tec, Schiedlberg, Austria)", sensor_type="wet electrodes", reference="right earlobe", @@ -1778,9 +1781,9 @@ class Cattan2019_VR(BaseDataset): sensors=[ "Fp1", "Fp2", - "Fc5", + "FC5", "Fz", - "Fc6", + "FC6", "T7", "Cz", "T8", diff --git a/moabb/datasets/download.py b/moabb/datasets/download.py index b8fe1ef50..ee8debdb1 100644 --- a/moabb/datasets/download.py +++ b/moabb/datasets/download.py @@ -442,27 +442,109 @@ def _sourcedata_files_for_subject(target_dir, nemar_id, subject, force_update): raise NemarDownloadError( f"NEMAR dataset {nemar_id} has an unreadable sourcedata manifest." ) from exc + files = _manifest_subject_files(record, subject) + if files is None: + return None + if not files: + known = sorted( + { + str(entry["subject"]) + for entry in record.get("files") or [] + if isinstance(entry, dict) and entry.get("subject") + } + ) + raise NemarDownloadError( + f"NEMAR dataset {nemar_id} lists no sourcedata for subject " + f"{subject!r}. Known subjects: {known}." + ) + # Manifest names are literal file paths, but they are consumed as glob + # patterns (nemar's include and the local verification below) -- escape + # them so names containing [, ], * or ? still match. + return ["sourcedata/" + glob_module.escape(name) for name in files] - entries = record.get("files") or [] + +def _sourcedata_store_holds_data(target_dir): + """Whether a deposit's ``sourcedata/`` holds anything besides its manifest. + + The manifest lands inside the store, so it does not count: a store holding + only a manifest is not a fetched one. + + Uses :func:`os.walk` and looks only at ``filenames``. ``Path.rglob`` would + also match directories, so an interrupted fetch that left empty ``sub-*`` + directories behind would count as a fetched store. + """ + manifest_name = PurePosixPath(SOURCEDATA_PROVENANCE).name + for _root, _dirs, filenames in os.walk(target_dir / "sourcedata"): + if any(name != manifest_name for name in filenames): + return True + return False + + +def _manifest_subject_files(record, subject): + """The files a provenance record attributes to ``subject``. + + ``None`` when the manifest predates the ``subject`` field and so cannot + answer per subject; an empty list when it can answer but knows nothing + about this subject. Names are returned raw -- callers that feed them to a + glob escape them themselves. + """ + entries = [entry for entry in (record.get("files") or []) if isinstance(entry, dict)] if not any("subject" in entry for entry in entries): return None candidates = subject if isinstance(subject, (list, tuple, set)) else [subject] wanted = {str(candidate) for candidate in candidates} - files = [ + return [ entry["file"] for entry in entries if entry.get("file") and str(entry.get("subject")) in wanted ] - if not files: - raise NemarDownloadError( - f"NEMAR dataset {nemar_id} lists no sourcedata for subject " - f"{subject!r}. Known subjects: " - f"{sorted({str(e.get('subject')) for e in entries if e.get('subject')})}." + + +def _is_stored_file(sourcedata_dir, name): + """Whether ``name`` from a manifest names a real file inside the store. + + Manifest names are data, so they are not trusted to stay relative: an + absolute one would make ``/`` discard the store root entirely, and a + directory must not pass for a downloaded file. + """ + return not Path(name).is_absolute() and (sourcedata_dir / name).is_file() + + +def nemar_sourcedata_is_local(target_dir, subject): + """Whether a subject's ``sourcedata/`` is already on disk, without any network. + + Answering per subject is what lets a caller loading one subject at a time + skip only what it really has; when the manifest cannot answer, a store that + already holds data is trusted as-is. + + This has to settle offline because ``nemar.download`` walks index, version, + metadata and manifest before it ever consults ``trust_existing`` -- a call + with nothing to do still costs four round-trips. + + Parameters + ---------- + target_dir : pathlib.Path + The deposit's local root, as returned by :func:`nemar_store`. Taken + already resolved because resolving it re-reads MNE's config file, which + a per-subject caller would otherwise pay for on every subject. + subject : int | str | list + Subject identifier, or the identifiers a deposit may file it under. + """ + target_dir = Path(target_dir) + try: + record = json.loads( + (target_dir / SOURCEDATA_PROVENANCE).read_text(encoding="utf-8") ) - # Manifest names are literal file paths, but they are consumed as glob - # patterns (nemar's include and the local verification below) -- escape - # them so names containing [, ], * or ? still match. - return ["sourcedata/" + glob_module.escape(name) for name in files] + files = _manifest_subject_files(record, subject) + except (OSError, ValueError, AttributeError, TypeError): + # Unreadable, or a manifest whose schema has drifted -- either way it + # cannot answer, so degrade to the whole-store rule rather than raising + # out of get_data(). + files = None + if files is None: + return _sourcedata_store_holds_data(target_dir) + sourcedata_dir = target_dir / "sourcedata" + return bool(files) and all(_is_stored_file(sourcedata_dir, name) for name in files) @verbose @@ -580,10 +662,7 @@ def nemar_sourcedata_dl( else: # The manifest does not count: a deposit that publishes only its # provenance has no original distribution to offer. - manifest = target_dir / SOURCEDATA_PROVENANCE - fetched = sourcedata_dir.is_dir() and any( - entry != manifest for entry in sourcedata_dir.rglob("*") - ) + fetched = _sourcedata_store_holds_data(target_dir) if not fetched: raise NemarDownloadError(missing) return str(sourcedata_dir) diff --git a/moabb/datasets/phmd_ml.py b/moabb/datasets/phmd_ml.py index 1628e45e1..8e77663ae 100644 --- a/moabb/datasets/phmd_ml.py +++ b/moabb/datasets/phmd_ml.py @@ -77,8 +77,8 @@ class Cattan2019_PHMD(BaseDataset): filters="no digital filter", sensors=[ "Cz", - "Fc5", - "Fc6", + "FC5", + "FC6", "Fp1", "Fp2", "Fz", @@ -172,7 +172,7 @@ def __init__(self, subjects=None, sessions=None): sessions_per_subject=1, events={"off": 1, "on": 2}, code="Cattan2019-PHMD", # Before: "PHMD-ML" - interval=[0, 1], + interval=[0, 60], paradigm="rstate", doi="10.5281/zenodo.2617084", selected_subjects=subjects, @@ -181,9 +181,9 @@ def __init__(self, subjects=None, sessions=None): self._chnames = [ "Fp1", "Fp2", - "Fc5", + "FC5", "Fz", - "Fc6", + "FC6", "T7", "Cz", "T8", @@ -216,6 +216,7 @@ def _get_single_subject_data(self, subject): ch_names=self._chnames, sfreq=512, ch_types=self._chtypes, verbose=False ) raw = mne.io.RawArray(data=X, info=info, verbose=False) + raw.set_montage("standard_1020") return {"0": {"0": raw}} def data_path( diff --git a/moabb/datasets/utils.py b/moabb/datasets/utils.py index 9aa2f9221..14db2b103 100644 --- a/moabb/datasets/utils.py +++ b/moabb/datasets/utils.py @@ -12,7 +12,6 @@ import zipfile from pathlib import Path -import matplotlib.pyplot as plt import mne import mne_bids import numpy as np @@ -21,11 +20,6 @@ from mne.io import RawArray import moabb.datasets as db -from moabb.analysis.plotting import ( - _get_dataset_parameters, - dataset_bubble_plot, - get_dataset_area, -) from moabb.datasets import download as dl from moabb.datasets._channel_pick import pick_channels_for_modalities # noqa: F401 from moabb.datasets.base import BaseDataset @@ -792,6 +786,12 @@ def get_centers(self): class _BaseDatasetPlotter: def __init__(self, datasets, meta_gap, kwargs, n_col=None): + # Imported here: moabb.analysis.plotting applies a seaborn theme to the + # global matplotlib rcParams at import time, which would restyle anyone + # who merely imports moabb.datasets. + + from moabb.analysis.plotting import _get_dataset_parameters, get_dataset_area + self.datasets = datasets = ( datasets if datasets is not None @@ -830,6 +830,13 @@ def _get_centers(self) -> np.ndarray: pass def plot(self): + # Imported here: moabb.analysis.plotting applies a seaborn theme to the + # global matplotlib rcParams at import time, which would restyle anyone + # who merely imports moabb.datasets. + import matplotlib.pyplot as plt + + from moabb.analysis.plotting import dataset_bubble_plot + centers = self._get_centers() rm = self.radii + self.meta_gap diff --git a/moabb/tests/test_download.py b/moabb/tests/test_download.py index 618a0c06a..80a17b878 100644 --- a/moabb/tests/test_download.py +++ b/moabb/tests/test_download.py @@ -1213,3 +1213,50 @@ def test_store_lookup_falls_back_to_url_tails_when_fname_differs(tmp_path, monke assert Path(path).read_text() == "mirrored" assert Path(path).name == "sub08_Testing1.mat" # destination keeps fname + + +def _fake_store(tmp_path): + root = dl.nemar_store("FAKE", "nm000902", str(tmp_path)) + (root / "sourcedata").mkdir(parents=True) + return root + + +def _is_local(root, subject): + return dl.nemar_sourcedata_is_local(root, subject) + + +def test_nemar_sourcedata_is_local_settles_presence_per_subject( + tmp_path, _isolated_mne_config +): + """One subject in the store must not vouch for the others.""" + root = _fake_store(tmp_path) + (root / dl.SOURCEDATA_PROVENANCE).write_text( + json.dumps( + { + "files": [ + {"file": "sub-01/eeg.set", "subject": "1"}, + {"file": "sub-02/eeg.set", "subject": "2"}, + ] + } + ) + ) + + # The manifest lands inside the store, but it is not data. + assert not _is_local(root, 1) + assert not _is_local(root, 2) + + # Fetching subject 1 must not make subject 2 look present. + (root / "sourcedata" / "sub-01").mkdir() + (root / "sourcedata" / "sub-01" / "eeg.set").write_text("mirrored") + assert _is_local(root, 1) + assert not _is_local(root, 2) + + +def test_nemar_sourcedata_is_local_trusts_a_store_without_a_manifest( + tmp_path, _isolated_mne_config +): + """Without a manifest there is nothing to reason with, so trust the store.""" + root = _fake_store(tmp_path) + assert not _is_local(root, 1) + (root / "sourcedata" / "foo.mat").write_text("mirrored") + assert _is_local(root, 1) diff --git a/moabb/tests/test_import_side_effects.py b/moabb/tests/test_import_side_effects.py new file mode 100644 index 000000000..9d6afa5cb --- /dev/null +++ b/moabb/tests/test_import_side_effects.py @@ -0,0 +1,63 @@ +"""Importing moabb must not restyle the caller's matplotlib. + +``moabb.analysis.plotting`` themes the global rcParams at import time. That is +fine for someone who asked for plotting, but it used to reach anyone who merely +imported a dataset, via ``moabb.datasets.bids_interface`` -> ``analysis.results``. +""" + +import json +import subprocess +import sys + +import pytest + + +# Read at axes creation, so a caller cannot undo them afterwards. +_WATCHED = ("font.family", "axes.grid", "axes.spines.left") + +_PROBE = """ +import json, sys, matplotlib +matplotlib.use("Agg") +import matplotlib.pyplot as plt +before = {{k: repr(plt.rcParams[k]) for k in {watched!r}}} +import {module} # noqa: F401 +after = {{k: repr(plt.rcParams[k]) for k in {watched!r}}} +json.dump([before, after], sys.stdout) +""" + + +def _around_import(module): + """rcParams before and after importing ``module`` in a fresh interpreter.""" + out = subprocess.run( + [sys.executable, "-c", _PROBE.format(module=module, watched=_WATCHED)], + capture_output=True, + text=True, + check=True, + ) + return json.loads(out.stdout) + + +def test_importing_moabb_does_not_restyle_matplotlib(): + for module in ("moabb", "moabb.datasets", "moabb.analysis"): + before, after = _around_import(module) + assert before == after, f"import {module} mutated rcParams: {before} -> {after}" + + # The theme must be kept, not lost -- only stopped from leaking. + before, after = _around_import("moabb.analysis.plotting") + assert before != after + + +def test_lazily_exported_names_still_resolve(): + import moabb.analysis + + for name in ("codecarbon_plot", "distribution_plot", "emissions_summary"): + assert callable(getattr(moabb.analysis, name)) and name in dir(moabb.analysis) + + # The eager import used to bind the submodule as an attribute; deferring it + # must not take that away. + assert moabb.analysis.plotting.__name__ == "moabb.analysis.plotting" + assert "plotting" in dir(moabb.analysis) + + # Exercise the __getattr__ this PR added, not the interpreter's own. + with pytest.raises(AttributeError, match="definitely_not_real"): + _ = moabb.analysis.definitely_not_real