-
Notifications
You must be signed in to change notification settings - Fork 256
Apply cross-validation settings consistently #1130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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, | ||
|
Comment on lines
+89
to
91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a within-session evaluation has Useful? React with 👍 / 👎. |
||
| ) | ||
| } | ||
| 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, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a custom cross-validator constructor accepts forwarded options through
**kwargsrather than naming every parameter, this filter incorrectly treats all defaults as incompatible. For example, aGroupKFoldsubclass with__init__(self, **kwargs)can acceptn_splits, butCrossSubjectEvaluation(n_splits=3, cv_class=Subclass)drops that value here and silently uses the subclass's underlying default fold count instead. Detect a variadic keyword parameter and retain compatible defaults in that case.Useful? React with 👍 / 👎.