Skip to content

Conversation

@malcolmbarrett
Copy link
Contributor

@malcolmbarrett malcolmbarrett commented Jan 19, 2019

Closes #112

This PR adds a tidy() method for model_fit objects. Right now, it uses a tryCatch to grab the generics message (which seems about the right message to me). I could always remove that and let the generic handle it, since it's a pretty readable message on its own. Currently, the tidy.model_fit object is exported, which is a bit out of norm, so I can change that if needed.

library(tidymodels)
library(parsnip)

logistic_reg() %>%
  set_engine("glm") %>%
  fit(Class ~ funded_amnt + int_rate, data = lending_club) %>%
  tidy(conf.int = TRUE, exponentiate = TRUE)
#> # A tibble: 3 x 7
#>   term        estimate  std.error statistic   p.value conf.low conf.high
#>   <chr>          <dbl>      <dbl>     <dbl>     <dbl>    <dbl>     <dbl>
#> 1 (Intercept)  169.    0.157         32.7   3.54e-235  125.      231.   
#> 2 funded_amnt    1.00  0.00000515     0.537 5.91e-  1    1.000     1.00 
#> 3 int_rate       0.853 0.00846      -18.7   1.99e- 78    0.839     0.868

rand_forest(mode = "classification") %>%
  set_engine("randomForest") %>%
  fit(Class ~ funded_amnt + int_rate, data = lending_club) %>%
  tidy()
#> Error: No tidy method for objects of class randomForest

@topepo
Copy link
Member

topepo commented Jan 20, 2019

The new tibble seems to have a version conflict for rlang. Can you change the description file to require rlang (>= 0.3.1 ) ?

@topepo
Copy link
Member

topepo commented Feb 25, 2019

The example fails with:

> logistic_reg() %>%
+   set_engine("glm") %>%
+   fit(Class ~ funded_amnt + int_rate, data = lending_club) %>%
+   # tidying model object and passing arguments to broom:::tidy.glm
+   tidy(conf.int = TRUE, exponentiate = TRUE)
Error in confint.glm(x, level = conf.level) : 
  package 'MASS' must be installed
Calls: %>% ... suppressMessages -> withCallingHandlers -> <Anonymous> -> confint.glm

Is there something else that you could demo with so that we wouldn't have to add MASS as a dependency?

@malcolmbarrett
Copy link
Contributor Author

Wow, this is a buried one. stats:::confint.glm depends on MASS, so let me try removing the conf.int argument here

@malcolmbarrett
Copy link
Contributor Author

Oh, a follow-up issue: tidy()is in generics, but tidy.glm() is in broom. Putting it in suggests will still fail with a documentation example, right? I could add to depends, but that's a big one, too. Any way to have an example and avoid this?

@topepo
Copy link
Member

topepo commented Feb 28, 2019

Oy vey. Can we do without an example? I'm trying to keep the dependencies low and broom would blow that up (even in suggests; travis times out).

@malcolmbarrett
Copy link
Contributor Author

Yeah, it's a big one, so I get that! I also feel like the example is nice but not essential given the context, so I removed it

@codecov-io
Copy link

codecov-io commented Feb 28, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@9b28f37). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #121   +/-   ##
=========================================
  Coverage          ?   72.72%           
=========================================
  Files             ?       36           
  Lines             ?     2651           
  Branches          ?        0           
=========================================
  Hits              ?     1928           
  Misses            ?      723           
  Partials          ?        0
Impacted Files Coverage Δ
R/tidy.R 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b28f37...3dd5747. Read the comment docs.

@topepo topepo merged commit 52ddb78 into tidymodels:master Mar 1, 2019
@topepo
Copy link
Member

topepo commented Mar 1, 2019

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 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 Mar 8, 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.

add tidy methods for model_fit objects

4 participants