Skip to content

Fix WithinSession/WithinSubjectSplitter overwriting explicit n_splits in cv_kwargs#1107

Merged
bruAristimunha merged 5 commits into
developfrom
copilot/fix-n-splits-overwrite
Jun 27, 2026
Merged

Fix WithinSession/WithinSubjectSplitter overwriting explicit n_splits in cv_kwargs#1107
bruAristimunha merged 5 commits into
developfrom
copilot/fix-n-splits-overwrite

Conversation

Copilot AI commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

WithinSessionSplitter and WithinSubjectSplitter unconditionally injected n_folds/shuffle/random_state into the inner CV kwargs, clobbering values the caller explicitly passed via cv_kwargs. This made it impossible to request, e.g., a single stratified holdout split without an inner-CV wrapper that hides n_splits.

from sklearn.model_selection import StratifiedShuffleSplit
from moabb.evaluations.splitters import WithinSessionSplitter

# Previously gave 5 folds (n_splits clobbered by n_folds); now yields one 75/25 split per session
sp = WithinSessionSplitter(cv_class=StratifiedShuffleSplit, n_splits=1,
                           test_size=0.25, shuffle=True, random_state=42)

Changes

  • moabb/evaluations/splitters.py — In both splitters, only inject the n_folds/shuffle/random_state defaults when the key is absent from cv_kwargs (if p in params and p not in cv_kwargs). Caller-provided values now take precedence; defaults are unchanged when not supplied.
  • Docstrings — Note that explicit n_splits/shuffle/random_state in cv_kwargs override the corresponding constructor arguments.
  • moabb/tests/test_splits.py — Parametrized regression test asserting an explicit n_splits=1 survives and produces one split per group for both splitters.
  • docs/source/whats_new.rst — Changelog entry.

Copilot AI changed the title [WIP] Fix WithinSessionSplitter overwrite of explicit n_splits Fix WithinSession/WithinSubjectSplitter overwriting explicit n_splits in cv_kwargs Jun 27, 2026
Copilot AI requested a review from bruAristimunha June 27, 2026 20:32
@bruAristimunha bruAristimunha marked this pull request as ready for review June 27, 2026 21:05
…hinSubjectSplitter reproducible

- WithinSessionEvaluation/WithinSubjectEvaluation now map the base-class
  n_splits to the inner n_folds instead of hardcoding 5 folds, matching
  CrossSubjectEvaluation.
- WithinSubjectSplitter.split() reseeds its RNG per call (shared across
  subjects to preserve the legacy fold sequence) so repeated calls with a
  fixed random_state are reproducible.
- Correct the cv_kwargs docstrings/changelog: only n_splits can be passed
  through cv_kwargs (shuffle/random_state are named constructor params).
@bruAristimunha bruAristimunha enabled auto-merge (squash) June 27, 2026 23:08
@bruAristimunha bruAristimunha disabled auto-merge June 27, 2026 23:14
@bruAristimunha bruAristimunha merged commit 535a4b3 into develop Jun 27, 2026
11 of 12 checks passed
@bruAristimunha bruAristimunha deleted the copilot/fix-n-splits-overwrite branch June 27, 2026 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants