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

Deprecate gather.rset #233

Merged
merged 4 commits into from
Apr 16, 2021
Merged

Deprecate gather.rset #233

merged 4 commits into from
Apr 16, 2021

Conversation

juliasilge
Copy link
Member

Closes #140

This PR deprecates the gather.rset method and advises users to use pivot_longer() directly:

library(rsample)
library(tidyverse)
cv_obj <- vfold_cv(mtcars, v = 10)
cv_obj$lm_rmse <- rnorm(10, mean = 2)
cv_obj$nnet_rmse <- rnorm(10, mean = 1)

cv_obj %>%
  select(-splits) %>%
  pivot_longer(-id)
#> # A tibble: 20 x 3
#>    id     name       value
#>    <chr>  <chr>      <dbl>
#>  1 Fold01 lm_rmse   2.69  
#>  2 Fold01 nnet_rmse 1.99  
#>  3 Fold02 lm_rmse   2.60  
#>  4 Fold02 nnet_rmse 0.554 
#>  5 Fold03 lm_rmse   1.28  
#>  6 Fold03 nnet_rmse 0.856 
#>  7 Fold04 lm_rmse   2.20  
#>  8 Fold04 nnet_rmse 2.84  
#>  9 Fold05 lm_rmse   2.16  
#> 10 Fold05 nnet_rmse 2.07  
#> 11 Fold06 lm_rmse   1.90  
#> 12 Fold06 nnet_rmse 0.524 
#> 13 Fold07 lm_rmse   3.20  
#> 14 Fold07 nnet_rmse 0.0222
#> 15 Fold08 lm_rmse   0.954 
#> 16 Fold08 nnet_rmse 0.140 
#> 17 Fold09 lm_rmse   0.204 
#> 18 Fold09 nnet_rmse 0.318 
#> 19 Fold10 lm_rmse   2.33  
#> 20 Fold10 nnet_rmse 0.131

Created on 2021-04-15 by the reprex package (v2.0.0)

For now, let's not create a new collect_ function to get these kinds of results and see if this is a requested feature from users.

@juliasilge juliasilge requested a review from hfrick April 15, 2021 18:01
Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I noticed that the lifecycle vignette also suggests updating the examples and the tests. That might be a nice addition so that people see the select(-split) in action?

@juliasilge
Copy link
Member Author

Great idea @hfrick! I don't think I want to change the tests for this at this stage of only warning (and I don't want to test tidyr functionality) but adding the example is a good call.

@juliasilge juliasilge merged commit 6b1fe49 into master Apr 16, 2021
@juliasilge juliasilge deleted the deprecate-gather-rset branch April 16, 2021 16:49
@github-actions
Copy link

github-actions bot commented May 1, 2021

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.

@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2021
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 this pull request may close these issues.

replace gather.rset
2 participants