leeper / prediction

Tidy, Type-Safe 'prediction()' Methods
https://cran.r-project.org/package=prediction
Other
89 stars 14 forks source link

bugfix prediction_glm.R newdata argument #33

Closed vincentarelbundock closed 5 years ago

vincentarelbundock commented 5 years ago

This is a minor bugfix (with a new test) for: https://github.com/leeper/prediction/issues/32

I'm not sure how your version numbers work, so I just added a new entry to NEWS.

library(prediction)
data(mtcars)

# `data` with lm works
mod <- lm(am ~ hp + drat, data = mtcars)
prediction(mod, data = data.frame(hp = 110, drat = 3.9))
#> Data frame with 1 predictions from
#>  lm(formula = am ~ hp + drat, data = mtcars)
#> with average prediction: 0.5947

# `data` with glm doesn't work
mod <- glm(am ~ hp + drat, data = mtcars)
prediction(mod, data = data.frame(hp = 110, drat = 3.9))
#> Warning in model$family$mu.eta(predictions_link) * model_mat: longer object
#> length is not a multiple of shorter object length
#> Error in is.data.frame(x): dims [product 3] do not match the length of object [32]

This problem was caused by calling predict(data = ) instead of predict(newdata = in a couple places in prediction_glm.R.

Please ensure the following before submitting a PR:

vincentarelbundock commented 5 years ago

@leeper When you have a second, do you think you could take a look at this one? This PR would allow me to fix Travis failures and finish up work downstream in margins for my ggplot WIP PR.

I only modified 2 lines of actual code and added a test, so it shouldn't be very difficult to review. I also added my name as contributor, but this is so minor that I would be happy to remove it.

Travis works in 5 of the 6 cases; the only case where it fails relates to package installation in an old R release, so it has nothing to do with this commit.

leeper commented 5 years ago

Looks great. Thanks!

vincentarelbundock commented 5 years ago

Thanks!