CRAN Check Failure for Upcoming broom Release

simonpcouch commented 4 years ago

Hi there! The broom dev team just ran reverse dependency checks on the upcoming broom 0.7.0 release and found new errors/test failures for the CRAN version of this package. I've pasted the results below:

checking tests ...

 ERROR
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:

  SI1     9 114304.420 104924.680 123684.159  9379.739
  sex|male    0.080 -92.0%      -2.522     0.163 -15.447  < .001 ***
  2nd female      0.779 0.712 0.833
  1st female      0.896 0.856 0.926
  1st female 29.000      0.919 0.880 0.945
  ══ testthat results  ═══════════════════════════════════════════════════════════
  [ OK: 24 | SKIPPED: 0 | WARNINGS: 0 | FAILED: 1 ]
  1. Failure: regress (@test_stats.R#23) 

  Error: testthat unit tests failed
  In addition: Warning messages:
  1: package 'lubridate' was built under R version 3.6.2 
  2: package 'tidyr' was built under R version 3.6.2 
  Execution halted

I believe the following line in R/logistic.R is likely the cause of the issue:

model <- broom::augment(x$model, type.predict = "response") %>% as.data.frame()

augment.lm() no longer takes a type.predict argument, so the supplied argument will be ignored in the new version.

We hope to submit this new version of the package to CRAN in the coming weeks. If you encounter any problems fixing these issues, please feel free to reach out!🙂

vnijs commented 4 years ago

Thanks for reaching out @simonpcouch. Is there documentation on why this change was made (i.e., why will type.predict = "response" be removed)?

vnijs commented 4 years ago

@simonpcouch The issue does not have anything to do with broom::augment as far as I can tell. But rather with changes in broom::glance. I noticed that glance now shows nobs (nice!) but it also reports a different value for df. Can you confirm this was intentional? If so, I can remove adjustments in my code to get the correct df values for the F-test in linear regression.

> mod <- lm(mpg ~ vs + cyl, data = mtcars)
> broom::glance(mod)
# A tibble: 1 x 11
  r.squared adj.r.squared sigma statistic       p.value    df logLik   AIC   BIC deviance df.residual
      <dbl>         <dbl> <dbl>     <dbl>         <dbl> <int>  <dbl> <dbl> <dbl>    <dbl>       <int>
1     0.728         0.710  3.25      38.9 0.00000000623     3  -81.5  171.  177.     306.          29
> packageVersion("broom")
[1] '0.5.5'

> mod <- lm(mpg ~ vs + cyl, data = mtcars)
> broom::glance(mod)
# A tibble: 1 x 12
  r.squared adj.r.squared sigma statistic       p.value    df logLik   AIC   BIC deviance df.residual  nobs
      <dbl>         <dbl> <dbl>     <dbl>         <dbl> <dbl>  <dbl> <dbl> <dbl>    <dbl>       <int> <int>
1     0.728         0.710  3.25      38.9 0.00000000623     2  -81.5  171.  177.     306.          29    32
> packageVersion("broom")
[1] '0.7.0.9000'

simonpcouch commented 4 years ago

As for the augment.lm() no longer passing on the type.predict argument to its predict method, I'm having second thoughts--type.predict would now be passed through ... due to broader augment() rewrite, but I think it currently being ignored might be a bug. Filing an issue and will get back to you.

There is more discussion on the change of degrees of freedom in tidymodels/broom#273 and related issues. Hope this helps clarify.🙂

vnijs commented 4 years ago

Seems like type.predict is working as expected in the current broom master branch. I think I'm going to wait until the new version of broom is on CRAN and then update radiant.model with the appropriate requirements.

> model <- broom::augment(x$model, type.predict = "response")
> print(head(model))

# A tibble: 6 x 12
  survived pclass sex       age sibsp parch .fitted .resid .std.resid    .hat .sigma   .cooksd
  <lgl>    <fct>  <fct>   <dbl> <int> <int>   <dbl>  <dbl>      <dbl>   <dbl>  <dbl>     <dbl>
1 TRUE     1st    female 29         0     0   0.940  0.352      0.352 0.00306  0.968 0.0000281
2 TRUE     1st    male    0.917     1     2   0.752  0.755      0.762 0.0205   0.968 0.00100  
3 FALSE    1st    female  2         1     2   0.974 -2.70      -2.70  0.00300  0.964 0.0161   
4 FALSE    1st    male   30         1     2   0.490 -1.16      -1.17  0.0134   0.967 0.00189  
5 FALSE    1st    female 25         1     2   0.937 -2.35      -2.36  0.00373  0.965 0.00806  
6 TRUE     1st    male   48         0     0   0.366  1.42       1.42  0.00629  0.967 0.00158  

> model <- broom::augment(x$model)
> print(head(model))

# A tibble: 6 x 12
  survived pclass sex       age sibsp parch .fitted .resid .std.resid    .hat .sigma   .cooksd
  <lgl>    <fct>  <fct>   <dbl> <int> <int>   <dbl>  <dbl>      <dbl>   <dbl>  <dbl>     <dbl>
1 TRUE     1st    female 29         0     0  2.75    0.352      0.352 0.00306  0.968 0.0000281
2 TRUE     1st    male    0.917     1     2  1.11    0.755      0.762 0.0205   0.968 0.00100  
3 FALSE    1st    female  2         1     2  3.62   -2.70      -2.70  0.00300  0.964 0.0161   
4 FALSE    1st    male   30         1     2 -0.0401 -1.16      -1.17  0.0134   0.967 0.00189  
5 FALSE    1st    female 25         1     2  2.71   -2.35      -2.36  0.00373  0.965 0.00806  
6 TRUE     1st    male   48         0     0 -0.551   1.42       1.42  0.00629  0.967 0.00158  
Waiting for profiling to be done...

simonpcouch commented 4 years ago

0.7.0 is up on CRAN! Thanks for your patience!

vnijs commented 4 years ago

Updates submitted to CRAN

radiant-rstats / radiant.model

CRAN Check Failure for Upcoming broom Release #5