Add a function model_get_coefficients_type() about broom.helpers HOT 16 CLOSED

I just published a first draft. Still have to add tests.

model_get_coefficients_type() is a method, so easy to extend. It will simply return "generic", "logistic", "poisson" or "prop_hazard". Its behaviour is very similar to the first part of gtsummary:::estimate_header().
It should be easy to use within gtsummary.

I have added the support of multinom model and of clm/clmm ordinal models.

tidy_add_coefficients_type() will add the type of coefficients as an attribute called coefficients_type as well as a second attribute coefficients_label equal to "Beta", "exp(Beta)", "OR", "log(OR)", "IRR", "log(IRR)", "HR" or "log(HR)". It remains simple: no translation and no footnote.

For gtsummary, I feel that the easiest would be to use model_get_coefficients_type() and then to use your custom code as you manage also translation and footnotes.

from broom.helpers.

OK. Could you have a look at c50ad38 and 7129ff6 ?

A new internal function .save_attributes is applied at the beginning of each tidy_* function to save selected attributes (defined only once).

At the end of each tidy_* function, tidy_attach_model() is applied. tidy_attach_model() now accepts a list of attributes to be attached. In addition, column sort has been moved within tidy_attach_model() and is performed before reattaching attributes.

Would it solve the problems you encounter?

from broom.helpers.

By the way, it could be relevant to also pass exponentiate as an attribute to model.

from broom.helpers.

This could be very useful for gtsummary and it would avoid duplication of effort. What do you have in mind? An additional column indicating the family (e.g. Poisson, Binomial, Gaussian, etc), or something more specific?

from broom.helpers.

I will quickly draft something. As these two informations (model_type and coefficients_type) are global, I'm thinking about adding attributes to the tibble

from broom.helpers.

Also adding support of quasibinomial and quasipoisson

from broom.helpers.

and polr models

from broom.helpers.

Very Big Picture Question:

I am about to submit a pull request for mice model support. I was having troubles with the model attribute being stripped and I had to update the function that re-orders the columns to also re-attach the model object. Generally, attributes to data frames are tricky and are often dropped when we don't intend that to happen. I am now experiencing the same problem with the exponentiate attribute just added.

Would it make more sense to return a list with the tidy tibble, the model object, and now the exponentiate value, rather than tacking them on as attributes? This would also make it easier to update in the future as we wouldn't need to worry about lost attributes.

from broom.helpers.

That's a very good question.

My first feeling is that a list would be easier for us as developers, but less convenient for users. tidy_*() functions are expected to return a tibble.

An alternative would be to provide a function for saving attributes at the beginning of each tidy_*() and to resave them at the end (e.g. with an attributes_list argument added to tidy_and_attach()).

I do not understand your need to update .order_tidy_columns() to reattached the model. In all tidy_ functions, the model is reattached just before applying .order_tidy_columns(). Do you have cases, where .order_tidy_columns() lose the attributes?

In that case, at the end of tidy_* functions, we should maybe apply .order_tidy_columns() before tidy_and_attach()

from broom.helpers.

My first feeling is that a list would be easier for us as developers, but less convenient for users. tidy_*() functions are expected to return a tibble.

I 100% agree with this statement! I think I was selfishly viewing the package from the point of view of a developer.

An alternative would be to provide a function for saving attributes at the beginning of each tidy_*() and to resave them at the end (e.g. with an attributes_list argument added to tidy_and_attach()).

This is a great option. If there is a single place that would need updating

Here's the example where the ordering stripped the model attribute

library(broom.helpers)

# imputing data
imputed_trial <- mice::mice(gtsummary::trial, m = 2)
# building model
mod <- with(imputed_trial, lm(age ~ marker + grade))

# creating tidy table with model attribute
x <- mice::pool(mod) %>% mice::tidy()
attr(x, "model") <- mod$analyses[[1]]

# this is a copy of the function that orders the columns
# the model attr is stripped in the select
x %>%
  dplyr::select(
    dplyr::any_of(
      c(
        "y.level", "term", "variable", "var_label", "var_class", "var_type",
        "header_row", "contrasts", "reference_row", "label"
      )
    ),
    dplyr::everything()
  ) %>%
  attr("model")
#> NULL

^{Created on 2020-10-04 by the reprex package (v0.3.0)}

from broom.helpers.

Bonus question: do you need that I transform tidy_and_attach() into a method, allowing to develop specific version for certain types of models?

from broom.helpers.

I think that will solve it all, and make this kind of issue future proof!

from broom.helpers.

Regarding the original topic (i.e. model_get_coefficients_type() and tidy_add_coefficients_type()), do you have any additional comment? Or can I close the issue for now?

For your information, compared to current gtsummary:::estimate_header(), I have added the detection of quasibinomial and quasipoisson models, polr models, multinom model and ordinal clm / clmm models.

from broom.helpers.

Bonus question: do you need that I transform tidy_and_attach() into a method, allowing to develop specific version for certain types of models?

What I had done here was make a new method function model_get_model() that grabbed the proper model object. I thought the new function fit better in the existing framework since none of the tidy_*() functions are methods, and there are many methods in the model_*() functions.

You'll see it soon, and if you don't like the new method function, I can update

from broom.helpers.

Regarding the original topic (i.e. model_get_coefficients_type() and tidy_add_coefficients_type()), do you have any additional comment? Or can I close the issue for now?

For your information, compared to current gtsummary:::estimate_header(), I have added the detection of quasibinomial and quasipoisson models, polr models, multinom model and ordinal clm / clmm models.

I didn't look at the results yet, but it sounds fantastic!

from broom.helpers.

Thanks

from broom.helpers.