Skip to content

glmnet multi_predict(): Check type for all modes #900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 14, 2023
Merged

glmnet multi_predict(): Check type for all modes #900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec900af
check type for all modes
hfrick Mar 2, 2023
40c24be
bump version in anticipation of #897 being merged first
hfrick Mar 6, 2023
dc09350
update NEWS
hfrick Mar 6, 2023
55ef2ba
Merge branch 'main' into glmnet-multi_predict-type-check
hfrick Mar 13, 2023
f01f555
update to merge PR
hfrick Mar 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: parsnip
Title: A Common API to Modeling and Analysis Functions
Version: 1.0.4.9002
Version: 1.0.4.9003
Authors@R: c(
person("Max", "Kuhn", , "max@posit.co", role = c("aut", "cre")),
person("Davis", "Vaughan", , "davis@posit.co", role = "aut"),
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

* Several internal functions (to help work with `Surv` objects) were added as a standalone file that can be used in other packages via `usethis::use_standalone("tidymodels/parsnip")`.

* `multi_predict()` methods for `linear_reg()`, `logistic_reg()`, and `multinomial_reg()` models fitted with the `"glmnet"` engine now check the `type` better and error accordingly (#900).

* Rather than being implemented in each method, the check for the `new_data` argument being mistakenly passed as `newdata` to `multi_predict()` now happens in the generic. Packages re-exporting the `multi_predict()` generic and implementing now-duplicate checks may see new failures and can remove their own analogous checks. This check already existed in all `predict()` methods (via `predict.model_fit()`) and all parsnip `multi_predict()` methods (#525).

* `logistic_reg()` will now warn at `fit()` when the outcome has more than two levels (#545).
Expand Down
16 changes: 8 additions & 8 deletions R/glmnet-engines.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,20 @@ multi_predict_glmnet <- function(object,
type = NULL,
penalty = NULL,
...) {
type <- check_pred_type(object, type)
check_spec_pred_type(object, type)
if (type == "prob") {
check_spec_levels(object)
}

dots <- list(...)

if (object$spec$mode == "classification") {
if (is_quosure(penalty)) {
penalty <- eval_tidy(penalty)
}
}

dots <- list(...)

object$spec <- eval_args(object$spec)

if (is.null(penalty)) {
Expand All @@ -195,12 +201,6 @@ multi_predict_glmnet <- function(object,
model_type <- class(object$spec)[1]

if (object$spec$mode == "classification") {
if (is.null(type)) {
type <- "class"
}
if (!(type %in% c("class", "prob", "link", "raw"))) {
rlang::abort("`type` should be either 'class', 'link', 'raw', or 'prob'.")
}
if (type == "prob" |
model_type == "logistic_reg") {
dots$type <- "response"
Expand Down