[ENH] splitter that replicates loc of another splitter
#4851
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fkiraly
added
module:forecasting
fkiraly
requested review from
achieveordie,
benHeid and
yarnabrina
as code owners
|
this is weird, equality tests fails only on windows, as array type seems to be |
This was referenced Jul 9, 2023
|
review comments addressed, kindly re-review |
benHeid
pushed a commit
that referenced
this pull request
Jul 14, 2023
benHeid
previously approved these changes
Jul 14, 2023
fkiraly
added a commit
that referenced
this pull request
Jul 15, 2023
…not equal length (#4861) This refactors the `evaluate` forecasting benchmark tool and adds features: * refactors the internal `_split` to use the `BaseSplitter` interface instead of separate logic. * to handle `X`, we use the `SameLocSplitter` from #4851 and the `TestPlusTrainSplitter` from #4862 * As #4851 is more general and allows `y` and `X` of different length, this fixes #4842 * new argument to `evaluate` which allows the user to pass the splitter for `X` explicitly as `cv_X`. If not passed, defaults to the `SameLocSplitter`, i.e., `X` split indices are same as from `y`. (requires no deprecation as added as last arg, and default is existing behaviour) * proper docstring Note: this changes behaviour in cases where `y` and `X` had same length but different index set. However, I would contend (is this correct?) that in these cases behaviour was generally unexpected or buggy, so no deprecation is needed. Depends on: #4851 #4862 Related to, and indirectly fixes #4842 Includes MRE from #4842 as an integration test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a splitter that takes a splitter and some data, and always replicates the same
locreferences for splits of another splitter.Related issue and discussion: #4842
More general form of
temporal_train_test_split, particularly useful for the case where we want to splitXandy, and primarilyy, andXcan have different indices fromy.This ability partially addresses #4842, FYI @felipeangelimvieira