easystats / insight

:crystal_ball: Easy access to model information for various model objects
https://easystats.github.io/insight/
GNU General Public License v3.0
395 stars 39 forks source link

Important: `get_predicted.default` needs default value for `predict` argument #547

Closed vincentarelbundock closed 2 years ago

vincentarelbundock commented 2 years ago

Otherwise, any call like get_predicted(model) will generate an error.

The default should be predict = "expectation"

https://github.com/easystats/insight/blob/master/R/get_predicted.R#L167

strengejacke commented 2 years ago

hm, probably too late for the current update.

strengejacke commented 2 years ago

Should be resolved in #549 Note, however, that get_predicted(x, type = "response") will fail, because predict is not set to NULL now any longer. So whenever you pass type to the default method, you must explicitly set predict = NULL. The intention not to use a default value was that you instead need to explicitly set either predict or type, which I found less confusing...

bwiernik commented 2 years ago

Can we make it so that if both are present, type overrides predict?

strengejacke commented 2 years ago

Yes, I think so. I'm not sure why we decided to error when both are given...

bwiernik commented 2 years ago

Can we make it so that if both are present, type overrides predict?

@strengejacke Did the above change get implemented?

strengejacke commented 2 years ago

Yes, but just after CRAN submission. So it's only in the version on R Universe