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

Change conf_mat_resampled() return object for tidy = FALSE #370

Merged
merged 3 commits into from Apr 20, 2021

Conversation

juliasilge
Copy link
Member

Closes #256

This PR changes what conf_mat_resampled(tidy = FALSE) returns, so that it returns the same kind of conf_mat object as yardstick::conf_mat().

library(tidymodels)
#> Registered S3 method overwritten by 'tune':
#>   method                   from   
#>   required_pkgs.model_spec parsnip

data(two_class_dat, package = "modeldata")

set.seed(2393)
res <-
  logistic_reg() %>%
  set_engine("glm") %>%
  fit_resamples(Class ~ ., resamples = vfold_cv(two_class_dat, v = 3),
                control = control_resamples(save_pred = TRUE))

conf_mat_resampled(res)
#> # A tibble: 4 x 3
#>   Prediction Truth   Freq
#>   <fct>      <fct>  <dbl>
#> 1 Class1     Class1 123  
#> 2 Class1     Class2  25.7
#> 3 Class2     Class1  22.7
#> 4 Class2     Class2  92.3
conf_mat_resampled(res, tidy = FALSE)
#>           Class1    Class2
#> Class1 123.00000  22.66667
#> Class2  25.66667  92.33333

conf_mat_resampled(res, tidy = FALSE) %>%
  autoplot()

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

@juliasilge
Copy link
Member Author

A question: 🤔 what do we think about the default of tidy = TRUE? It feels really different from the yardstick function/results (where the conf_mat results have a tidy() method), in a surprising and maybe inconsistent way.

Is the best option keeping it this way, at least for now? We've had this function since tune 0.1.1 so a switch to FALSE may be disruptive at this point.

@juliasilge juliasilge requested a review from topepo April 19, 2021 23:16
@topepo
Copy link
Member

topepo commented Apr 20, 2021

I agree about the tidy argument but I've got to submit tomorrow so let's keep it.

@topepo topepo merged commit e0452d6 into master Apr 20, 2021
@topepo topepo deleted the conf-mat-resampled branch April 20, 2021 00:32
@github-actions
Copy link

github-actions bot commented May 5, 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 5, 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.

conf_mat_resampled doesn't have a plotting method
2 participants