From 8397de8b89e27e7ab5aec1bcd3af75ebcca3ce33 Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Tue, 22 Jun 2021 07:35:26 -0400 Subject: [PATCH 1/8] new extract functions --- DESCRIPTION | 3 ++- NAMESPACE | 6 ++++++ R/extract.R | 48 ++++++++++++++++++++++++++++++++++++++++++ R/reexports.R | 8 +++++++ man/extract-parsnip.Rd | 41 ++++++++++++++++++++++++++++++++++++ man/reexports.Rd | 4 ++++ 6 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 R/extract.R create mode 100644 man/extract-parsnip.Rd diff --git a/DESCRIPTION b/DESCRIPTION index f3c07f7db..a445cb384 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -31,7 +31,8 @@ Imports: tidyr (>= 1.0.0), globals, prettyunits, - vctrs (>= 0.2.0) + vctrs (>= 0.2.0), + hardhat (>= 0.1.5.9000) Roxygen: list(markdown = TRUE) RoxygenNote: 7.1.1.9001 Suggests: diff --git a/NAMESPACE b/NAMESPACE index a7e92493f..0d039bfbc 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,8 @@ # Generated by roxygen2: do not edit by hand S3method(augment,model_fit) +S3method(extract_fit_engine,model_fit) +S3method(extract_parsnip_spec,model_fit) S3method(fit,model_spec) S3method(fit_xy,gen_additive_mod) S3method(fit_xy,model_spec) @@ -138,6 +140,8 @@ export(control_parsnip) export(convert_stan_interval) export(decision_tree) export(eval_args) +export(extract_fit_engine) +export(extract_parsnip_spec) export(fit) export(fit.model_spec) export(fit_control) @@ -255,6 +259,8 @@ importFrom(generics,required_pkgs) importFrom(generics,tidy) importFrom(generics,varying_args) importFrom(glue,glue_collapse) +importFrom(hardhat,extract_fit_engine) +importFrom(hardhat,extract_parsnip_spec) importFrom(magrittr,"%>%") importFrom(purrr,as_vector) importFrom(purrr,imap) diff --git a/R/extract.R b/R/extract.R new file mode 100644 index 000000000..c43d4fa3d --- /dev/null +++ b/R/extract.R @@ -0,0 +1,48 @@ +#' Extract elements of a parsnip model object +#' +#' @description +#' These functions extract various elements from a parsnip object. If they do +#' not exist yet, an error is thrown. +#' +#' - `extract_parsnip_spec()` returns the parsnip model specification. +#' +#' - `extract_fit_engine()` returns the engine specific fit embedded within +#' a parsnip model fit. For example, when using [parsnip::linear_reg()] +#' with the `"lm"` engine, this would return the underlying `lm` object. +#' +#' @param x A parsnip `fit.model_spec()` object. +#' @param ... Not currently used. +#' @return +#' The extracted value from the parsnip object, `x`, as described in the description +#' section. +#' +#' @name extract-parsnip +#' @examples +#' lm_spec <- linear_reg() %>% set_engine("lm") +#' lm_fit <- fit(lm_spec, mpg ~ ., data = mtcars) +#' +#' lm_spec +#' extract_parsnip_spec(lm_fit) +#' +#' extract_fit_engine(lm_fit) +#' lm(mpg ~ ., data = mtcars) +NULL + +#' @export +#' @rdname extract-parsnip +extract_parsnip_spec.model_fit <- function(x, ...) { + if (any(names(x) == "spec")) { + return(x$spec) + } + rlang::abort("The model fit does not have a model spec.") +} + + +#' @export +#' @rdname extract-parsnip +extract_fit_engine.model_fit <- function(x, ...) { + if (any(names(x) == "fit")) { + return(x$fit) + } + rlang::abort("The model fit does not have an engine fit.") +} diff --git a/R/reexports.R b/R/reexports.R index c6fa3d9f3..cffe9db0e 100644 --- a/R/reexports.R +++ b/R/reexports.R @@ -26,3 +26,11 @@ generics::augment #' @importFrom generics required_pkgs #' @export generics::required_pkgs + +#' @importFrom hardhat extract_parsnip_spec +#' @export +hardhat::extract_parsnip_spec + +#' @importFrom hardhat extract_fit_engine +#' @export +hardhat::extract_fit_engine diff --git a/man/extract-parsnip.Rd b/man/extract-parsnip.Rd new file mode 100644 index 000000000..2218e6d05 --- /dev/null +++ b/man/extract-parsnip.Rd @@ -0,0 +1,41 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/extract.R +\name{extract-parsnip} +\alias{extract-parsnip} +\alias{extract_parsnip_spec.model_fit} +\alias{extract_fit_engine.model_fit} +\title{Extract elements of a parsnip model object} +\usage{ +\method{extract_parsnip_spec}{model_fit}(x, ...) + +\method{extract_fit_engine}{model_fit}(x, ...) +} +\arguments{ +\item{x}{A parsnip \code{fit.model_spec()} object.} + +\item{...}{Not currently used.} +} +\value{ +The extracted value from the parsnip object, \code{x}, as described in the description +section. +} +\description{ +These functions extract various elements from a parsnip object. If they do +not exist yet, an error is thrown. +\itemize{ +\item \code{extract_parsnip_spec()} returns the parsnip model specification. +\item \code{extract_fit_engine()} returns the engine specific fit embedded within +a parsnip model fit. For example, when using \code{\link[=linear_reg]{linear_reg()}} +with the \code{"lm"} engine, this would return the underlying \code{lm} object. +} +} +\examples{ +lm_spec <- linear_reg() \%>\% set_engine("lm") +lm_fit <- fit(lm_spec, mpg ~ ., data = mtcars) + +lm_spec +extract_parsnip_spec(lm_fit) + +extract_fit_engine(lm_fit) +lm(mpg ~ ., data = mtcars) +} diff --git a/man/reexports.Rd b/man/reexports.Rd index 614b0b3b9..f072cc382 100644 --- a/man/reexports.Rd +++ b/man/reexports.Rd @@ -10,6 +10,8 @@ \alias{glance} \alias{augment} \alias{required_pkgs} +\alias{extract_parsnip_spec} +\alias{extract_fit_engine} \alias{varying_args} \title{Objects exported from other packages} \keyword{internal} @@ -20,6 +22,8 @@ below to see their documentation. \describe{ \item{generics}{\code{\link[generics]{augment}}, \code{\link[generics]{fit}}, \code{\link[generics]{fit_xy}}, \code{\link[generics]{glance}}, \code{\link[generics]{required_pkgs}}, \code{\link[generics]{tidy}}, \code{\link[generics]{varying_args}}} + \item{hardhat}{\code{\link[hardhat:hardhat-extract]{extract_fit_engine}}, \code{\link[hardhat:hardhat-extract]{extract_parsnip_spec}}} + \item{magrittr}{\code{\link[magrittr:pipe]{\%>\%}}} }} From b965e25f95c3fa53a9b50786f346ef3a84450515 Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Tue, 22 Jun 2021 07:43:25 -0400 Subject: [PATCH 2/8] missing hardhat remote --- DESCRIPTION | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index a445cb384..bf2b1292f 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -58,4 +58,5 @@ Suggests: Matrix, mgcv Remotes: - topepo/C5.0 + topepo/C5.0, + tidymodels/hardhat@feature/extract-generics From 7cfa87f02cef45b264f5e9190b997b724b53ef9f Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Thu, 24 Jun 2021 13:19:20 -0400 Subject: [PATCH 3/8] extract_parsnip_spec -> extract_spec_parsnip --- NAMESPACE | 6 +++--- R/extract.R | 6 +++--- R/reexports.R | 4 ++-- man/extract-parsnip.Rd | 8 ++++---- man/reexports.Rd | 4 ++-- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index 0d039bfbc..778e8b1c3 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -2,7 +2,7 @@ S3method(augment,model_fit) S3method(extract_fit_engine,model_fit) -S3method(extract_parsnip_spec,model_fit) +S3method(extract_spec_parsnip,model_fit) S3method(fit,model_spec) S3method(fit_xy,gen_additive_mod) S3method(fit_xy,model_spec) @@ -141,7 +141,7 @@ export(convert_stan_interval) export(decision_tree) export(eval_args) export(extract_fit_engine) -export(extract_parsnip_spec) +export(extract_spec_parsnip) export(fit) export(fit.model_spec) export(fit_control) @@ -260,7 +260,7 @@ importFrom(generics,tidy) importFrom(generics,varying_args) importFrom(glue,glue_collapse) importFrom(hardhat,extract_fit_engine) -importFrom(hardhat,extract_parsnip_spec) +importFrom(hardhat,extract_spec_parsnip) importFrom(magrittr,"%>%") importFrom(purrr,as_vector) importFrom(purrr,imap) diff --git a/R/extract.R b/R/extract.R index c43d4fa3d..d24ba61a9 100644 --- a/R/extract.R +++ b/R/extract.R @@ -4,7 +4,7 @@ #' These functions extract various elements from a parsnip object. If they do #' not exist yet, an error is thrown. #' -#' - `extract_parsnip_spec()` returns the parsnip model specification. +#' - `extract_spec_parsnip()` returns the parsnip model specification. #' #' - `extract_fit_engine()` returns the engine specific fit embedded within #' a parsnip model fit. For example, when using [parsnip::linear_reg()] @@ -22,7 +22,7 @@ #' lm_fit <- fit(lm_spec, mpg ~ ., data = mtcars) #' #' lm_spec -#' extract_parsnip_spec(lm_fit) +#' extract_spec_parsnip(lm_fit) #' #' extract_fit_engine(lm_fit) #' lm(mpg ~ ., data = mtcars) @@ -30,7 +30,7 @@ NULL #' @export #' @rdname extract-parsnip -extract_parsnip_spec.model_fit <- function(x, ...) { +extract_spec_parsnip.model_fit <- function(x, ...) { if (any(names(x) == "spec")) { return(x$spec) } diff --git a/R/reexports.R b/R/reexports.R index cffe9db0e..65a0025ef 100644 --- a/R/reexports.R +++ b/R/reexports.R @@ -27,9 +27,9 @@ generics::augment #' @export generics::required_pkgs -#' @importFrom hardhat extract_parsnip_spec +#' @importFrom hardhat extract_spec_parsnip #' @export -hardhat::extract_parsnip_spec +hardhat::extract_spec_parsnip #' @importFrom hardhat extract_fit_engine #' @export diff --git a/man/extract-parsnip.Rd b/man/extract-parsnip.Rd index 2218e6d05..e3a8737f8 100644 --- a/man/extract-parsnip.Rd +++ b/man/extract-parsnip.Rd @@ -2,11 +2,11 @@ % Please edit documentation in R/extract.R \name{extract-parsnip} \alias{extract-parsnip} -\alias{extract_parsnip_spec.model_fit} +\alias{extract_spec_parsnip.model_fit} \alias{extract_fit_engine.model_fit} \title{Extract elements of a parsnip model object} \usage{ -\method{extract_parsnip_spec}{model_fit}(x, ...) +\method{extract_spec_parsnip}{model_fit}(x, ...) \method{extract_fit_engine}{model_fit}(x, ...) } @@ -23,7 +23,7 @@ section. These functions extract various elements from a parsnip object. If they do not exist yet, an error is thrown. \itemize{ -\item \code{extract_parsnip_spec()} returns the parsnip model specification. +\item \code{extract_spec_parsnip()} returns the parsnip model specification. \item \code{extract_fit_engine()} returns the engine specific fit embedded within a parsnip model fit. For example, when using \code{\link[=linear_reg]{linear_reg()}} with the \code{"lm"} engine, this would return the underlying \code{lm} object. @@ -34,7 +34,7 @@ lm_spec <- linear_reg() \%>\% set_engine("lm") lm_fit <- fit(lm_spec, mpg ~ ., data = mtcars) lm_spec -extract_parsnip_spec(lm_fit) +extract_spec_parsnip(lm_fit) extract_fit_engine(lm_fit) lm(mpg ~ ., data = mtcars) diff --git a/man/reexports.Rd b/man/reexports.Rd index f072cc382..fccef00df 100644 --- a/man/reexports.Rd +++ b/man/reexports.Rd @@ -10,7 +10,7 @@ \alias{glance} \alias{augment} \alias{required_pkgs} -\alias{extract_parsnip_spec} +\alias{extract_spec_parsnip} \alias{extract_fit_engine} \alias{varying_args} \title{Objects exported from other packages} @@ -22,7 +22,7 @@ below to see their documentation. \describe{ \item{generics}{\code{\link[generics]{augment}}, \code{\link[generics]{fit}}, \code{\link[generics]{fit_xy}}, \code{\link[generics]{glance}}, \code{\link[generics]{required_pkgs}}, \code{\link[generics]{tidy}}, \code{\link[generics]{varying_args}}} - \item{hardhat}{\code{\link[hardhat:hardhat-extract]{extract_fit_engine}}, \code{\link[hardhat:hardhat-extract]{extract_parsnip_spec}}} + \item{hardhat}{\code{\link[hardhat:hardhat-extract]{extract_fit_engine}}, \code{\link[hardhat:hardhat-extract]{extract_spec_parsnip}}} \item{magrittr}{\code{\link[magrittr:pipe]{\%>\%}}} }} From 9720b7153f157eea98e2c531e0208666a9fa0a8d Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Fri, 25 Jun 2021 16:35:26 -0400 Subject: [PATCH 4/8] notes about model extraction --- R/extract.R | 18 ++++++++++++++++++ man/extract-parsnip.Rd | 15 +++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/R/extract.R b/R/extract.R index d24ba61a9..7cd780134 100644 --- a/R/extract.R +++ b/R/extract.R @@ -12,6 +12,24 @@ #' #' @param x A parsnip `fit.model_spec()` object. #' @param ... Not currently used. +#' @details +#' Extracting the underlying engine fit can be helpful for describing the +#' model via `print()`, `summarize()`, `plot()`, and so on. +#' +#' However, users should not invoke the `predict()` method on an extracted +#' model. There may be preprocessing operations that `parsnip` has executed on +#' the data prior to giving it to the model. Bypassing these can lead to errors +#' or silently generating incorrect predictions. +#' +#' **Good**: +#' ```r +#' parsnip_fit %>% predict(new_data) +#' ``` +#' +#' **Bad**: +#' ```r +#' parsnip_fit %>% extract_fit_engine() %>% predict(new_data) +#' ``` #' @return #' The extracted value from the parsnip object, `x`, as described in the description #' section. diff --git a/man/extract-parsnip.Rd b/man/extract-parsnip.Rd index e3a8737f8..c7557cde0 100644 --- a/man/extract-parsnip.Rd +++ b/man/extract-parsnip.Rd @@ -29,6 +29,21 @@ a parsnip model fit. For example, when using \code{\link[=linear_reg]{linear_reg with the \code{"lm"} engine, this would return the underlying \code{lm} object. } } +\details{ +Extracting the underlying engine fit can be helpful for describing the +model via \code{print()}, \code{summarize()}, and so on. + +However, users should not invoke the \code{predict()} method on an extracted +model. There may be preprocessing operations that \code{parsnip} has executed on +the data prior to giving it to the model. Bypassing these can lead to errors +or silently generating incorrect predictions. + +\strong{Good}:\if{html}{\out{
}}\preformatted{ parsnip_fit \%>\% predict(new_data) +}\if{html}{\out{
}} + +\strong{Bad}:\if{html}{\out{
}}\preformatted{ parsnip_fit \%>\% extract_fit_engine() \%>\% predict(new_data) +}\if{html}{\out{
}} +} \examples{ lm_spec <- linear_reg() \%>\% set_engine("lm") lm_fit <- fit(lm_spec, mpg ~ ., data = mtcars) From 0d60fcbd257b8c665936ff5df3ad3ccbbd6cf7e7 Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Sat, 26 Jun 2021 09:17:15 -0400 Subject: [PATCH 5/8] small text adjustments --- R/extract.R | 5 +++-- man/extract-parsnip.Rd | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/R/extract.R b/R/extract.R index 7cd780134..8c174bff3 100644 --- a/R/extract.R +++ b/R/extract.R @@ -10,11 +10,12 @@ #' a parsnip model fit. For example, when using [parsnip::linear_reg()] #' with the `"lm"` engine, this would return the underlying `lm` object. #' -#' @param x A parsnip `fit.model_spec()` object. +#' @param x A parsnip `model_fit` object. #' @param ... Not currently used. #' @details #' Extracting the underlying engine fit can be helpful for describing the -#' model via `print()`, `summarize()`, `plot()`, and so on. +#' model (via `print()`, `summary()`, `plot()`, etc.) or for variable +#' importance/explainers. #' #' However, users should not invoke the `predict()` method on an extracted #' model. There may be preprocessing operations that `parsnip` has executed on diff --git a/man/extract-parsnip.Rd b/man/extract-parsnip.Rd index c7557cde0..f1e30a0ea 100644 --- a/man/extract-parsnip.Rd +++ b/man/extract-parsnip.Rd @@ -11,7 +11,7 @@ \method{extract_fit_engine}{model_fit}(x, ...) } \arguments{ -\item{x}{A parsnip \code{fit.model_spec()} object.} +\item{x}{A parsnip \code{model_fit} object.} \item{...}{Not currently used.} } @@ -31,17 +31,18 @@ with the \code{"lm"} engine, this would return the underlying \code{lm} object. } \details{ Extracting the underlying engine fit can be helpful for describing the -model via \code{print()}, \code{summarize()}, and so on. +model (via \code{print()}, \code{summary()}, \code{plot()}, etc.) or for variable +importance/explainers. However, users should not invoke the \code{predict()} method on an extracted model. There may be preprocessing operations that \code{parsnip} has executed on the data prior to giving it to the model. Bypassing these can lead to errors or silently generating incorrect predictions. -\strong{Good}:\if{html}{\out{
}}\preformatted{ parsnip_fit \%>\% predict(new_data) +\strong{Good}:\if{html}{\out{
}}\preformatted{ parsnip_fit \%>\% predict(new_data) }\if{html}{\out{
}} -\strong{Bad}:\if{html}{\out{
}}\preformatted{ parsnip_fit \%>\% extract_fit_engine() \%>\% predict(new_data) +\strong{Bad}:\if{html}{\out{
}}\preformatted{ parsnip_fit \%>\% extract_fit_engine() \%>\% predict(new_data) }\if{html}{\out{
}} } \examples{ From 5ba9a4836bd0a38224cc628117c5e02940d409d3 Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Wed, 30 Jun 2021 16:29:07 -0400 Subject: [PATCH 6/8] Update R/extract.R Co-authored-by: Hannah Frick --- R/extract.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/extract.R b/R/extract.R index 8c174bff3..55f7fd84d 100644 --- a/R/extract.R +++ b/R/extract.R @@ -8,7 +8,7 @@ #' #' - `extract_fit_engine()` returns the engine specific fit embedded within #' a parsnip model fit. For example, when using [parsnip::linear_reg()] -#' with the `"lm"` engine, this would return the underlying `lm` object. +#' with the `"lm"` engine, this returns the underlying `lm` object. #' #' @param x A parsnip `model_fit` object. #' @param ... Not currently used. From 69253efa0600599a774964a9e9ca82254dc29659 Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Wed, 30 Jun 2021 16:40:29 -0400 Subject: [PATCH 7/8] extract test cases --- R/extract.R | 4 ++-- tests/testthat/test-extract.R | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 tests/testthat/test-extract.R diff --git a/R/extract.R b/R/extract.R index 55f7fd84d..c26c47fb0 100644 --- a/R/extract.R +++ b/R/extract.R @@ -53,7 +53,7 @@ extract_spec_parsnip.model_fit <- function(x, ...) { if (any(names(x) == "spec")) { return(x$spec) } - rlang::abort("The model fit does not have a model spec.") + rlang::abort("Internal error: The model fit does not have a model spec.") } @@ -63,5 +63,5 @@ extract_fit_engine.model_fit <- function(x, ...) { if (any(names(x) == "fit")) { return(x$fit) } - rlang::abort("The model fit does not have an engine fit.") + rlang::abort("Internal error: The model fit does not have an engine fit.") } diff --git a/tests/testthat/test-extract.R b/tests/testthat/test-extract.R new file mode 100644 index 000000000..9a03b1f70 --- /dev/null +++ b/tests/testthat/test-extract.R @@ -0,0 +1,19 @@ + +context("model extraction") + +# ------------------------------------------------------------------------------ + +test_that('extract', { + x <- linear_reg() %>% set_engine("lm") %>% fit(mpg ~ ., data = mtcars) + x_no_spec <- x + x_no_spec$spec <- NULL + x_no_fit <- x + x_no_fit$fit <- NULL + + expect_true(inherits(extract_spec_parsnip(x), "model_spec")) + expect_true(inherits(extract_fit_engine(x), "lm")) + + expect_error(extract_spec_parsnip(x_no_spec), "Internal error") + expect_error(extract_fit_engine(x_no_fit), "Internal error") +}) + From 2103c4cc917a47541e64203e149b881292545e4e Mon Sep 17 00:00:00 2001 From: Max Kuhn Date: Thu, 1 Jul 2021 19:24:59 -0400 Subject: [PATCH 8/8] less specific remote --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index e14b081d2..87fc49044 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -61,5 +61,5 @@ Suggests: Remotes: tidymodels/dials, topepo/C5.0, - tidymodels/hardhat@feature/extract-generics + tidymodels/hardhat