search

tidymodels / rsample

Classes and functions to create and summarize resampling objects
https://rsample.tidymodels.org
Other
341 stars 66 forks source link

New subclasses and `inner_split()` methods for validation sets #489

Closed hfrick closed 6 months ago

hfrick commented 6 months ago

This PR adds inner_split() methods for the rsets coming from validation_set(). Those rsets can result from the group and time variants of initial_validation_split().

To be able to tell where they are coming from, this PR also adds subclasses for the group and time variant to the output of validation_set(): the rset objects and the rsplit objects within those rsets.

I'm looking for a second opinion on the naming of the rsplit subclasses for the time variant.

The interfaces and the original (and deprecated) approach via validation_split() put the time neither as a prefix nor a suffix but rather inside the more general name: initial_time_split(), initial_validation_time_split(), validation_time_split(), and val_time_split.

I don't think we are going to touch the naming of the interfaces but we could touch the naming convention for the rsplit class. I've broken with that particular pattern of placing time and instead aligned with the pattern for group, i.e., used it as a prefix. Could be good, could be bad, could actually not matter much.

The only place I think it'll matter is here and I'm happy to add a method for val_time_split to support the usage of the deprecated validation_time_split() if we think it's necessary. Edit: this search suggests that we don't dispatch on that class elsewhere so it should only matter here.

github-actions[bot] commented 5 months ago

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.