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

Could get_rsplit() add a method for resample_results from tune::fit_resamples()? #687

Open
mikemahoney218 opened this issue May 22, 2023 · 2 comments
Labels
feature a feature request or enhancement

Comments

@mikemahoney218
Copy link
Member

The problem

In answering tidymodels/hardhat#240 , I was tempted to use the rsample::get_rsplit() helper function on the output from tune::fit_resamples() to avoid needing to use the $splits[[1]] syntax, which I think can be a bit confusing to folks not used to thinking about rset objects. However, the outputs from fit_resamples() are of classes 'resample_results', 'tune_results', 'tbl_df', 'tbl', 'data.frame', and get_rsplit() only handles objects of class rset(). Could get_rsplit() add a method for resample_results (or, alternatively, could the output from fit_resamples() also be an rset() object)?

Reproducible example

data("ames", package = "modeldata")
ames_sf <- sf::st_as_sf(ames, coords = c("Longitude", "Latitude"), crs = 4326)

recipe <- recipes::recipe(Sale_Price ~ Year_Built, data = sf::st_drop_geometry(ames_sf)) |> 
  recipes::step_log(recipes::all_outcomes())

workflows::workflow(recipe, parsnip::linear_reg()) |> 
  tune::fit_resamples(spatialsample::spatial_clustering_cv(ames_sf)) |> 
  rsample::get_rsplit(1)
#> Error in `rsample::get_rsplit()`:
#> ! No `get_rsplit()` method for this class(es) 'resample_results', 'tune_results', 'tbl_df', 'tbl', 'data.frame'
#> Backtrace:
#>     ▆
#>  1. ├─rsample::get_rsplit(...)
#>  2. └─rsample:::get_rsplit.default(...)
#>  3.   └─rlang::abort(...)

Created on 2023-05-22 with reprex v2.0.2

@hfrick hfrick transferred this issue from tidymodels/rsample May 22, 2023
@hfrick
Copy link
Member

hfrick commented May 22, 2023

I've moved this to tune because we usually put an S3 method for a class in the same package that owns the class. I wouldn't mind a get_rsplit.resample_results() but I'll defer to @topepo for the final answer 😄

@simonpcouch simonpcouch added the feature a feature request or enhancement label Oct 31, 2023
@jrosell
Copy link

jrosell commented Oct 9, 2024

One approach could be to first extract the resamples from the tune results like I suggested here #947 and the use rsample::get_rsplit on the rset generated object..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants