ENH: Allow top-level splitters in CrossSubjectEvaluation

[lindicaphxag-tech]

Closes #1088.

CrossSubjectEvaluation currently hardcodes its top-level CrossSubjectSplitter, so a metadata-aware cross-subject protocol cannot reuse MOABB's caching, scoring, joblib parallelism, result database, and fold metadata handling without reimplementing the evaluation engine.

This PR adds an optional splitter= injection point for a scikit-learn BaseCrossValidator following MOABB's existing split(y, metadata) contract.

Design constraints:

• Default CrossSubjectEvaluation behavior is unchanged when splitter=None.

• An explicit top-level splitter is mutually exclusive with the existing top-level protocol controls (cv_class, cv_kwargs, n_splits, groups, and non-default cs_mode) so two protocol definitions cannot silently conflict.

• Splitter-declared metadata_columns are carried into result rows, and per-fold get_metadata() output goes through the existing evaluation task path.

• Train/calibration/test indices are consumed by the same fold engine as the built-in cross-subject protocols. Explicit splitters must yield exactly 2 or 3 slices with unique, in-range, one-dimensional integer positions; train/calibration/test slices must be pairwise disjoint, so malformed custom protocols cannot silently leak trials.

• The positional contract is explicit because the evaluator slices metadata with iloc; custom splitters must not rely on DataFrame index labels.

• Each explicit test fold must resolve to exactly one subject. MOABB stores one subject identity per result row, so accepting a multi-subject test fold would silently misattribute the scientific unit being evaluated.

• Splitter metadata is deep-copied at fold materialization time, so a custom splitter that reuses/mutates nested metadata objects cannot retroactively change earlier folds' recorded provenance.

Validation:

• Explicit splitter construction and protocol-conflict checks.
• Splitter metadata propagation.
• End-to-end fake EEG cross-subject evaluation through the real caching/scoring pipeline.
• Non-RangeIndex metadata regression proving that splitter outputs are interpreted as positions.
• Rejection of multi-subject test folds.
• Rejection of overlapping, duplicate, non-integer, out-of-range, and wrong-arity custom fold indices.
• Frozen-head focused validation on 4a01cb02 independently ran uv run --no-sync python -m pytest moabb/tests/test_evaluations.py -q -k cross_subject_top_level_splitter on Python 3.11 → 14 passed, including the end-to-end fake-EEG evaluation and all malformed-fold contract checks (run #37115116998).
• Pre-commit CI is green; repository GitHub Actions currently require maintainer approval for this external contribution.

Scope boundary with grouped cross-subject CV

The explicit splitter= hook in this PR intentionally follows the single-target use case from #1104: a metadata-aware top-level splitter may exclude part of the held-out target subject (for example, test only one target session while dropping the target's other sessions), so each fold still has one target subject and one subject identity in the result rows.

This is distinct from the standard cv_class / grouped-CV path. Multi-subject test folds produced there are a valid separate use case; #1211 handles their result provenance by fitting once and scoring/storing each held-out subject separately. In other words, #1207 does not redefine grouped CV as single-subject-only; it keeps the new transfer/exclusion hook narrow to the issue that motivated it.

[lindicaphxag-tech]

Follow-up on the current head f6365603: this keeps the change focused on making explicit top-level splitters usable in CrossSubjectEvaluation while preserving MOABB's cache/result pipeline and fold metadata. The regressions also reject test folds that span multiple subjects so results cannot be silently attributed to the first subject. pre-commit.ci is green; the remaining upstream workflows are still waiting for fork-CI approval. Could a maintainer review/approve CI when convenient? I can address any requested changes.

[lindicaphxag-tech]

Self-review hardening on the current head: I made the splitter index contract explicit and executable. The parallel evaluator slices folds with metadata.iloc[...], so a top-level splitter must return positional integer indices, not DataFrame index labels. The test splitter now uses positions, and a regression deliberately gives metadata a non-RangeIndex to verify the returned indices still select the intended held-out subject. No evaluation semantics changed; this prevents a custom transfer splitter from working only by accident when metadata happens to have a default index.

[lindicaphxag-tech]

Protocol-safety hardening on the current head: explicit top-level splitters are now validated before any fold reaches the evaluator. Only 2-way (train, test) or 3-way (train, calibration, test) outputs are accepted; positions must be 1-D integer, unique, in range, and pairwise disjoint. This closes a leakage/contract gap where an overlapping split or a 4-way tuple could otherwise be partially consumed by the generic *cal unpacking. Built-in splitter behavior is unchanged because the strict index checks are enabled only for an explicitly injected CrossSubject splitter.

[lindicaphxag-tech]

Updated exact-head verification on 6a9ab07883960c8509f71b96373b86a8d696bf86: the complete moabb/tests/test_evaluations.py suite passes locally (152 passed, 5 skipped; skips are existing optional Optuna/skorch cases). The 16 focused splitter tests and pre-commit also pass. This run includes the latest per-fold nested-metadata snapshot/provenance regression; no code changed after the run.

The 3-way calibration case still goes through CrossSubjectEvaluation.process() with a transfer-aware pipeline consuming X_target_unlabeled; invalid fold arity, overlap, range, and subject-boundary cases fail before evaluation. Could a maintainer review this public splitter contract when available?

[lindicaphxag-tech]

Review-history cleanup only: I squashed the branch to a single commit before maintainer review. The Git tree is unchanged (1e9c58b67dc5df6f90b5becb13fa1a6ec961b6ae), so the existing focused/parity evidence remains applicable. I’ll hold this head unless review identifies a concrete issue.

[lindicaphxag-tech]

@codex review

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[bruAristimunha]

Thanks @lindicaphxag-tech. I checked the design against #1088: this is option 1 (splitter instance, additive, default unchanged) with the cross-subject-only scope, and the mutual exclusion against cv_class/cv_kwargs/n_splits/groups/non-default cs_mode removes the two-protocol ambiguity. Positional-index contract and one-subject-per-test-fold invariant match the existing .iloc/per-subject result-row semantics. splitter=None is preserved by the early return, and the _preview_splits refactor is equivalent for 2- and 3-tuple outputs. CI is green on aa8c877.

Two points I want to settle before approving, both about new global behaviour outside the splitter path:

1. _validate_test_fold_metadata now raises on an empty test fold for every evaluation type. I'm inclined to keep it as a hard error; speak up if you see a legitimate custom-CV case that emits empty folds.
2. _preview_splits now rejects 4+-tuple splitters instead of silently binning the middle elements into cal. Also fine by me, but it's an API change for third-party splitters and should be listed under API changes in whats_new.rst, not only as an enhancement.

Optional simplification: the five if kwargs.get(...) is not None: conflicts.append(...) blocks in evaluations.py could be one comprehension.

[lindicaphxag-tech]

Both review points are addressed on current head f9cb0544. I agree an empty test fold should remain a hard error: there is no meaningful result-row identity or score to emit for it, and silently accepting it would make custom-CV failures much harder to diagnose. I also added the requested API changes note stating that 4+-slice custom splitters are now rejected and that only (train, test) / (train, calibration, test) are supported. The current pre-commit.ci error is reported as error during mergeable check after develop moved, not as a hook failure. Per your requested merge order I’ll avoid another churn/rebase until #1206 is settled, then refresh this branch before merge.