From 5f8b23e3db5f5bc7e89c1362e3eace14b6c3a868 Mon Sep 17 00:00:00 2001 From: stanbot8 Date: Wed, 29 Jul 2026 20:27:42 -0700 Subject: [PATCH] fix: apply cross-validation settings --- docs/source/whats_new.rst | 2 ++ moabb/evaluations/base.py | 15 ++++++---- moabb/evaluations/evaluations.py | 47 +++++++++++++++----------------- moabb/tests/test_evaluations.py | 27 ++++++++++++++++++ 4 files changed, 60 insertions(+), 31 deletions(-) diff --git a/docs/source/whats_new.rst b/docs/source/whats_new.rst index f3160f032..dae416b57 100644 --- a/docs/source/whats_new.rst +++ b/docs/source/whats_new.rst @@ -81,6 +81,7 @@ Bugs - Fix missing electrode positions (NaN xyz) in six motor-imagery datasets so topographic maps, interpolation, and spatial methods work: :class:`moabb.datasets.Forenzo2023` and :class:`moabb.datasets.GuttmannFlury2025_MI`/``_ME`` normalize Neuroscan ALL_CAPS labels and apply ``standard_1005`` (CB1/CB2 kept as ``misc``); :class:`moabb.datasets.Dreyer2023` falls back to ``standard_1005`` when the BIDS archive ships no ``electrodes.tsv``; :class:`moabb.datasets.BNCI2003_004` maps its 26 legacy Berlin channel labels to their modern 10-5 equivalents for exact positions; :class:`moabb.datasets.BNCI2014_002` applies an approximate 3x5 grid for its unlabeled small-Laplacian channels; and :class:`moabb.datasets.Zhang2017` applies the ``GSN-HydroCel-32`` montage in EGI sensor order. Adds the shared :func:`moabb.datasets.utils.set_neuroscan_montage` helper (:gh:`1089` by `Bruno Aristimunha`_). - Fix ``BaseEvaluation._aggregate_fold_results`` aborting the whole evaluation with ``TypeError: agg function failed [how->mean,dtype->object]`` when a single fold contributes a non-numeric ``score`` (e.g. an error fold). The numeric aggregation columns are now coerced with ``pandas.to_numeric(errors="coerce")`` before ``groupby.agg``, so a bad fold becomes ``NaN`` and is skipped instead of taking down every subject/pipeline (:gh:`1095` by `Bruno Aristimunha`_). - Fix :class:`moabb.evaluations.splitters.WithinSessionSplitter` and :class:`moabb.evaluations.splitters.WithinSubjectSplitter` overwriting an explicit ``n_splits`` passed through ``cv_kwargs`` with the ``n_folds`` default; the caller-provided ``n_splits`` now takes precedence, so a single holdout split can be requested directly via ``cv_class=StratifiedShuffleSplit, n_splits=1``. :class:`moabb.evaluations.WithinSessionEvaluation` and :class:`moabb.evaluations.WithinSubjectEvaluation` now honour the ``n_splits`` argument instead of always running 5 folds, and :class:`moabb.evaluations.splitters.WithinSubjectSplitter` now yields reproducible per-subject folds for a fixed ``random_state`` (:gh:`1106` by `Bruno Aristimunha`_). +- Evaluations now apply ``cv_kwargs`` to the default cross-validation class. Caller settings override splitter defaults, and splitter construction passes each setting 1 time (by `Stanley C.`_). - Fix numeric sorting in the dataset summary tables (:doc:`dataset_summary`): columns containing the ``varies`` sentinel (e.g. ``Total_trials``) were auto-detected as strings by DataTables and sorted lexicographically (``11000 < 1114 < 11496``). A custom ``num-varies`` column type now treats such columns as numeric, sorting sentinel rows last while keeping their displayed text unchanged (:gh:`1118` by `Bhargav Kowshik`_). - Fix ``make html`` crash in ``scripts/generate_macro_table.py`` when a dataset has a missing (``NaN``) value in an optional metadata column (country, DOI, data URL, ...): the float ``NaN`` is truthy, so it slipped past the ``if not value`` guards and crashed the string formatters (``TypeError: object of type 'float' has no len()``). ``_format_cell`` now normalizes ``NaN`` to ``None`` before dispatching, and ``_dataset_link``/``_paradigm_tag`` -- the only two format branches without an empty-value guard, which raised ``AttributeError`` on ``html.escape(None)`` -- guard it too, so all eleven branches render a missing cell as empty (:gh:`1117` by `Bhargav Kowshik`_). @@ -923,3 +924,4 @@ API changes .. _Henrique Lefundes: https://github.com/HenriqueLefundes .. _Paul-Adrien Graignic: https://github.com/pagraignic-yneuro .. _pre-commit-ci: https://github.com/apps/pre-commit-ci +.. _Stanley C.: https://github.com/stanbot8 diff --git a/moabb/evaluations/base.py b/moabb/evaluations/base.py index dffb90831..4aa2e7527 100644 --- a/moabb/evaluations/base.py +++ b/moabb/evaluations/base.py @@ -1,3 +1,4 @@ +import inspect import logging import math from abc import ABC, abstractmethod @@ -541,12 +542,14 @@ def __init__( def _resolve_cv(self, default_class, default_kwargs=None): """Resolve the cross-validation class and kwargs for a splitter.""" - if self.cv_class is None: - cv_class = default_class - cv_kwargs = {} if default_kwargs is None else dict(default_kwargs) - else: - cv_class = self.cv_class - cv_kwargs = dict(self.cv_kwargs) + cv_class = default_class if self.cv_class is None else self.cv_class + cv_kwargs = {} if default_kwargs is None else dict(default_kwargs) + if self.cv_class is not None: + parameters = inspect.signature(cv_class).parameters + cv_kwargs = { + name: value for name, value in cv_kwargs.items() if name in parameters + } + cv_kwargs.update(self.cv_kwargs) return cv_class, cv_kwargs def _load_data( diff --git a/moabb/evaluations/evaluations.py b/moabb/evaluations/evaluations.py index cbc1559f9..0c090217e 100644 --- a/moabb/evaluations/evaluations.py +++ b/moabb/evaluations/evaluations.py @@ -84,15 +84,15 @@ class WithinSessionEvaluation(BaseEvaluation): def _create_splitter(self): """Create the WithinSessionSplitter for parallel evaluation.""" cv_class, cv_kwargs = self._resolve_cv(StratifiedKFold) - if self.groups is not None: - cv_kwargs = {**cv_kwargs, "groups": self.groups} - return WithinSessionSplitter( - n_folds=self.n_splits or 5, - shuffle=True, - random_state=self.random_state, - cv_class=cv_class, + splitter_kwargs = { + "n_folds": self.n_splits or 5, + "shuffle": True, + "random_state": self.random_state, **cv_kwargs, - ) + } + if self.groups is not None: + splitter_kwargs["groups"] = self.groups + return WithinSessionSplitter(cv_class=cv_class, **splitter_kwargs) # flake8: noqa: C901 def _evaluate( @@ -299,11 +299,10 @@ class CrossSessionEvaluation(BaseEvaluation): def _create_splitter(self): """Create the CrossSessionSplitter for parallel evaluation.""" cv_class, cv_kwargs = self._resolve_cv(LeaveOneGroupOut) + splitter_kwargs = {"random_state": self.random_state, **cv_kwargs} if self.groups is not None: - cv_kwargs = {**cv_kwargs, "groups": self.groups} - return CrossSessionSplitter( - cv_class=cv_class, random_state=self.random_state, **cv_kwargs - ) + splitter_kwargs["groups"] = self.groups + return CrossSessionSplitter(cv_class=cv_class, **splitter_kwargs) # flake8: noqa: C901 def evaluate( @@ -542,13 +541,11 @@ def _create_splitter(self): default_class = GroupKFold default_kwargs = {"n_splits": self.n_splits} - default_kwargs.update(self.cv_kwargs) cv_class, cv_kwargs = self._resolve_cv(default_class, default_kwargs) + splitter_kwargs = {"random_state": self.random_state, **cv_kwargs} if self.groups is not None: - cv_kwargs = {**cv_kwargs, "groups": self.groups} - return CrossSubjectSplitter( - cv_class=cv_class, random_state=self.random_state, **cv_kwargs - ) + splitter_kwargs["groups"] = self.groups + return CrossSubjectSplitter(cv_class=cv_class, **splitter_kwargs) def evaluate( self, @@ -643,15 +640,15 @@ class WithinSubjectEvaluation(BaseEvaluation): def _create_splitter(self): """Create the WithinSubjectSplitter for parallel evaluation.""" cv_class, cv_kwargs = self._resolve_cv(StratifiedKFold) - if self.groups is not None: - cv_kwargs = {**cv_kwargs, "groups": self.groups} - return WithinSubjectSplitter( - n_folds=self.n_splits or 5, - shuffle=True, - random_state=self.random_state, - cv_class=cv_class, + splitter_kwargs = { + "n_folds": self.n_splits or 5, + "shuffle": True, + "random_state": self.random_state, **cv_kwargs, - ) + } + if self.groups is not None: + splitter_kwargs["groups"] = self.groups + return WithinSubjectSplitter(cv_class=cv_class, **splitter_kwargs) def evaluate( self, diff --git a/moabb/tests/test_evaluations.py b/moabb/tests/test_evaluations.py index 028c7f9e4..6ea6a714c 100644 --- a/moabb/tests/test_evaluations.py +++ b/moabb/tests/test_evaluations.py @@ -12,6 +12,7 @@ from pyriemann.spatialfilters import CSP from sklearn.discriminant_analysis import LinearDiscriminantAnalysis as LDA from sklearn.dummy import DummyClassifier as Dummy +from sklearn.model_selection import GroupShuffleSplit from sklearn.pipeline import FunctionTransformer, Pipeline, make_pipeline from moabb.analysis.results import get_digest, get_string_rep @@ -487,6 +488,32 @@ def test_resolve_cv_honours_cv_kwargs_without_forcing_defaults(): os.remove(e.results.filepath) +def test_default_cv_kwargs_override_splitter_defaults(tmp_path): + evaluation = ev.WithinSessionEvaluation( + paradigm=FakeImageryParadigm(), + datasets=[dataset], + hdf5_path=tmp_path, + cv_kwargs={"n_splits": 3, "shuffle": False}, + ) + splitter = evaluation._create_splitter() + assert splitter._cv_kwargs["n_splits"] == 3 + assert splitter.shuffle is False + + +def test_custom_cv_receives_compatible_defaults_and_overrides(tmp_path): + evaluation = ev.CrossSubjectEvaluation( + paradigm=FakeImageryParadigm(), + datasets=[dataset], + hdf5_path=tmp_path, + n_splits=3, + cv_class=GroupShuffleSplit, + cv_kwargs={"random_state": 17}, + ) + splitter = evaluation._create_splitter() + assert splitter._cv_kwargs["n_splits"] == 3 + assert splitter.random_state == 17 + + class Test_CrossSubj(TestWithinSess): def setup_method(self): self.eval = ev.CrossSubjectEvaluation(