search

larmarange / broom.helpers

A set of functions to facilitate manipulation of tibbles produced by broom
https://larmarange.github.io/broom.helpers/
GNU General Public License v3.0
21 stars 8 forks source link

mmrm() models #229

Closed larmarange closed 9 months ago

larmarange commented 1 year ago

fix #228

ddsjoberg commented 1 year ago

Hey hey! I am getting an error trying to run tidy_plus_plus().

# this version is from the mmrm branch ("refs/heads/mmrm")
packageVersion("broom.helpers")
#> [1] '1.13.1'

fit <- 
  mmrm::mmrm(
    formula = FEV1 ~ RACE + SEX + ARMCD * AVISIT + us(AVISIT | USUBJID),
    data = mmrm::fev_data
  )
summary(fit)
#> mmrm fit
#> 
#> Formula:     FEV1 ~ RACE + SEX + ARMCD * AVISIT + us(AVISIT | USUBJID)
#> Data:        mmrm::fev_data (used 537 observations from 197 
#> subjects with maximum 4 timepoints)
#> Covariance:  unstructured (10 variance parameters)
#> Method:      Satterthwaite
#> Vcov Method: Asymptotic
#> Inference:   REML
#> 
#> Model selection criteria:
#>      AIC      BIC   logLik deviance 
#>   3406.4   3439.3  -1693.2   3386.4 
#> 
#> Coefficients: 
#>                                Estimate Std. Error        df t value Pr(>|t|)
#> (Intercept)                    30.77748    0.88656 218.80000  34.715  < 2e-16
#> RACEBlack or African American   1.53050    0.62448 168.67000   2.451 0.015272
#> RACEWhite                       5.64357    0.66561 157.14000   8.479 1.56e-14
#> SEXFemale                       0.32606    0.53195 166.13000   0.613 0.540744
#> ARMCDTRT                        3.77423    1.07415 145.55000   3.514 0.000589
#> AVISITVIS2                      4.83959    0.80172 143.88000   6.037 1.27e-08
#> AVISITVIS3                     10.34211    0.82269 155.56000  12.571  < 2e-16
#> AVISITVIS4                     15.05390    1.31281 138.47000  11.467  < 2e-16
#> ARMCDTRT:AVISITVIS2            -0.04193    1.12932 138.56000  -0.037 0.970439
#> ARMCDTRT:AVISITVIS3            -0.69369    1.18765 158.17000  -0.584 0.559996
#> ARMCDTRT:AVISITVIS4             0.62423    1.85085 129.72000   0.337 0.736463
#>                                  
#> (Intercept)                   ***
#> RACEBlack or African American *  
#> RACEWhite                     ***
#> SEXFemale                        
#> ARMCDTRT                      ***
#> AVISITVIS2                    ***
#> AVISITVIS3                    ***
#> AVISITVIS4                    ***
#> ARMCDTRT:AVISITVIS2              
#> ARMCDTRT:AVISITVIS3              
#> ARMCDTRT:AVISITVIS4              
#> ---
#> Signif. codes:  0 '***' 0.001 '**' 0.01 '*' 0.05 '.' 0.1 ' ' 1
#> 
#> Covariance estimate:
#>         VIS1    VIS2    VIS3    VIS4
#> VIS1 40.5537 14.3960  4.9747 13.3867
#> VIS2 14.3960 26.5715  2.7855  7.4745
#> VIS3  4.9747  2.7855 14.8979  0.9082
#> VIS4 13.3867  7.4745  0.9082 95.5568

broom.helpers::tidy_plus_plus(fit)
#> ! `broom::tidy()` failed to tidy the model.
#> mmrm() registered as emmeans extension
#> Warning: The `full` argument of `model.frame.mmrm_tmb()` is deprecated as of mmrm 0.3.
#> ℹ The deprecated feature was likely used in the insight package.
#>   Please report the issue at <https://github.com/easystats/insight/issues>.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.
#> ✔ `tidy_parameters()` used instead.
#> ℹ Add `tidy_fun = broom.helpers::tidy_parameters` to quiet these messages.
#> Error in stats::model.matrix.default(model %>% model_get_terms(), data = stats::model.frame(model)): model frame and formula mismatch in model.matrix()

Created on 2023-08-04 with reprex v2.0.2

I'll try to look into it further this weekend 🍁

larmarange commented 1 year ago

Just for you information, #228 has not been merged into the main branch yet.

larmarange commented 1 year ago

Ok. I do not have an issue on my side, but I'm using mmrm version 0.2.2, the last version available on CRAN.

It seems that there is some change with the dev version.

Should we wait for the stabilisation and the release of version 0.3.0 before updating that PR?

Regards

ddsjoberg commented 1 year ago

Should we wait for the stabilisation and the release of version 0.3.0 before updating that PR?

Great suggestion. The author is back from leave this coming week. Perhaps we can update mmrm to have all the needed methods and correct the weights() dimension issue, then update.

larmarange commented 1 year ago

Perfect. Let's wait for the release of 0.3.0 then

ddsjoberg commented 9 months ago

hey hey @larmarange ! hope you're well!

mmrm made their release to CRAN, and i bumped the min version requirement to the CRAN version.

Would you like me to try and take it from here, or you prefer to do it?

larmarange commented 9 months ago

hey hey @larmarange ! hope you're well!

mmrm made their release to CRAN, and i bumped the min version requirement to the CRAN version.

Would you like me to try and take it from here, or you prefer to do it?

Hey

I'm currently traveling for work. Don't hesitate to look / test the PR if you have time.

I could myself look at it next week

Best

larmarange commented 9 months ago

We have to check but with the new version of the package, some dedicated methods in broom.helpers won't be needed anymore

ddsjoberg commented 9 months ago

@larmarange the tests that are failing are related to lme4, rather than mmrm FYI

larmarange commented 9 months ago

This is strange. I just deleted all caches and I'm trying to re run all jobs

larmarange commented 9 months ago

Everything seems OK for me.

Test failures are related to https://stackoverflow.com/questions/77481539/error-in-initializeptr-function-cholmod-factor-ldeta-not-provided-by-pack and not to the PR

So, for me, not a problem to merge this PR.

However, we will have to fix all checks before next release

codecov[bot] commented 9 months ago

Codecov Report

Merging #229 (2c82160) into main (99294ff) will not change coverage. Report is 1 commits behind head on main. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/larmarange/broom.helpers/pull/229/graphs/tree.svg?width=650&height=150&src=pr&token=onefg3YI04&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange)](https://app.codecov.io/gh/larmarange/broom.helpers/pull/229?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange) ```diff @@ Coverage Diff @@ ## main #229 +/- ## ======================================= Coverage 95.48% 95.48% ======================================= Files 43 43 Lines 2680 2680 ======================================= Hits 2559 2559 Misses 121 121 ``` | [Files](https://app.codecov.io/gh/larmarange/broom.helpers/pull/229?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange) | Coverage Δ | | |---|---|---| | [R/model\_get\_weights.R](https://app.codecov.io/gh/larmarange/broom.helpers/pull/229?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Joseph+Larmarange#diff-Ui9tb2RlbF9nZXRfd2VpZ2h0cy5S) | `93.75% <ø> (ø)` | |