Skip to content

Conversation

@hfrick
Copy link
Member

@hfrick hfrick commented Feb 22, 2023

closes #877

predict_glmnet() calls predict.model_fit() which checks this via check_pred_type_dots() so we don't need it in predict_glmnet().

The predict_<type>() methods don't run this check. I think generally users are supposed to use predict(type = <type>) rather than predict_<type>() but the predict_<type>() functions are exported. If we think this check is important to have also when directly calling predict_<type>(), I'd suggest we add it to the method for model_fit objects but since it's not glmnet-specific, I have removed it here.

multi_predict() does not have a central model_fit method (because it doesn't apply to all models) so I have left the check in the glmnet-specific method.

The tests for glmnet engines live in extratests, those checks pass locally and will be run on CI once this is merged.

`predict.model_fit()` checks this via `check_pred_type_dots()`
@hfrick hfrick requested a review from EmilHvitfeldt February 22, 2023 14:49
Copy link
Member

@EmilHvitfeldt EmilHvitfeldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@hfrick hfrick merged commit 5dbeb21 into main Feb 23, 2023
@hfrick hfrick deleted the glmnet-newdata branch February 23, 2023 18:36
@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove checks on newdata typo in glmnet-related functions

3 participants