Skip to content
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

get_rsplit() helper to extract split objects? #389

Closed
mikemahoney218 opened this issue Dec 5, 2022 · 4 comments · Fixed by #399
Closed

get_rsplit() helper to extract split objects? #389

mikemahoney218 opened this issue Dec 5, 2022 · 4 comments · Fixed by #399

Comments

@mikemahoney218
Copy link
Member

Feature

Hi all!

Right now, I believe the best way for users to get individual rsplit objects from an rset is to index into the rset themselves:

library(rsample)

ex_vfold <- vfold_cv(Orange, v = 2)
ex_vfold$splits[[1]]
#> <Analysis/Assess/Total>
#> <17/18/35>

I'm wondering if it would be useful to provide a helper function, get_rsplit() or similar, to extract these? My personal interest is in making the spatialsample documentation a bit cleaner (because autoplot() has separate methods for rset and rsplit objects), but I think it might also be a friendlier interface overall:

get_rsplit <- function(rset, n, ...) {
  UseMethod("get_rsplit")
}

get_rsplit.rset <- function(rset, n, ...) rset$splits[[n]]

get_rsplit.default <- function(rset, n, ...) {
  cls <- paste0("'", class(x), "'", collapse = ", ")
  rlang::abort(
    paste("No `get_rsplit()` method for this class(es)", cls)
  )
}

get_rsplit(ex_vfold, 1)
#> <Analysis/Assess/Total>
#> <17/18/35>

Thanks!

@hfrick
Copy link
Member

hfrick commented Dec 6, 2022

I think this might not exist yet because rsplit objects (other than those from initial_split() and friends) are not generally meant to be used standalone.

That said, I wouldn't mind such functionality for illustration purposes. Do you want to make a PR for that?

If so, could you please

  • call the arg index instead of n
  • add a dot prefix to the name since it's more developer-focussed, so .get_rsplit()
  • add @keyword internal

@mikemahoney218
Copy link
Member Author

mikemahoney218 commented Dec 6, 2022

Happy to add a PR! That said, we use this pattern a lot in long-form user-facing documentation, across rsample, the tidymodels website, TMwR, and spatialsample (and possibly elsewhere, these are the only spots I checked):

https://rsample.tidymodels.org/articles/rsample.html#individual-resamples-are-rsplit-objects
https://rsample.tidymodels.org/articles/Applications/Recipes_and_rsample.html
https://rsample.tidymodels.org/articles/Working_with_rsets.html#model-assessment

https://www.tidymodels.org/learn/work/nested-resampling/
https://www.tidymodels.org/learn/models/time-series/

https://www.tmwr.org/resampling.html
https://www.tmwr.org/dimensionality.html

https://spatialsample.tidymodels.org/
https://spatialsample.tidymodels.org/articles/spatialsample.html
https://spatialsample.tidymodels.org/articles/buffering.html

Also once in parsnip:
https://parsnip.tidymodels.org/articles/Submodels.html

I think it's also in a good number of function examples.

So I'd suggest that this should be a user-facing helper as well, and not just a developer function (so no . prefix and no @keyword internal). Happy to make it internal if you disagree, though.

@hfrick
Copy link
Member

hfrick commented Dec 7, 2022

Oye! You got me convinced 😄 get_rsplit() it is!

@github-actions
Copy link

This issue 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.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants