Merge pull request #110 from tidymodels/lightgbm-params-warn

Author: EmilHvitfeldt

NEWS.md

@@ -1,5 +1,7 @@
 # bonsai (development version)
 
+* The lightgbm engine now warns that arguments passed to `params` argument in `set_engine()` are ignored (#110).
+
 * Automatic handling of `num_classes` argument when specifying a multiclass classification objective for the lightgbm engine (#109).
 
 * Increased the minimum R version to R 4.1.

R/lightgbm.R

@@ -100,6 +100,13 @@ train_lightgbm <- function(
       ...
     )
 
+  if (!is.null(args$params) && is.list(args$params)) {
+    cli::cli_warn(c(
+      "Arguments passed in through {.arg params} as a list will be ignored.",
+      "Instead pass the arguments directly to the {.code ...}."
+    ))
+  }
+
   args <- process_bagging(args)
   args <- process_objective_function(args, y)
 

tests/testthat/_snaps/lightgbm.md

@@ -44,6 +44,21 @@
       Error in `fit()`:
       ! `feature_fraction_bynode` must be a number, not a call.
 
+# lightgbm warns if user uses `param` argument in set_engine()
+
+    Code
+      fit(mod_spec, mpg ~ ., mtcars)
+    Condition
+      Warning:
+      Arguments passed in through `params` as a list will be ignored.
+      Instead pass the arguments directly to the `...`.
+    Output
+      parsnip model object
+      
+      LightGBM Model (1 tree)
+      Objective: regression
+      Fitted to dataset with 10 columns
+
 # training wrapper warns on protected arguments
 
     Code

tests/testthat/test-lightgbm.R

@@ -348,30 +348,6 @@ test_that("bonsai correctly determines objective when label is a factor", {
   expect_equal(bst$params$num_class, 3)
 })
 
-test_that("bonsai correctly determines num_classes when objective is set", {
-  skip_if_not_installed("lightgbm")
-  skip_if_not_installed("modeldata")
-
-  suppressPackageStartupMessages({
-    library(lightgbm)
-    library(dplyr)
-  })
-
-  data("penguins", package = "modeldata")
-  penguins <- penguins[complete.cases(penguins), ]
-
-  expect_no_error({
-    bst <- train_lightgbm(
-      x = penguins[, c("bill_length_mm", "bill_depth_mm")],
-      y = penguins[["species"]],
-      num_iterations = 5,
-      verbose = -1L,
-      objective = "multiclassova"
-    )
-  })
-  expect_identical(bst$params$objective, "multiclassova")
-  expect_identical(bst$params$num_class, 3L)
-})
 
 test_that("bonsai handles mtry vs mtry_prop gracefully", {
   skip_if_not_installed("modeldata")
@@ -535,6 +511,19 @@ test_that("tuning mtry vs mtry_prop", {
   )
 })
 
+test_that("lightgbm warns if user uses `param` argument in set_engine()", {
+  skip_if_not_installed("lightgbm")
+
+  mod_spec <- boost_tree() |>
+    set_engine("lightgbm", params = list(objective = "multiclassova")) |>
+    set_mode("regression")
+
+  expect_snapshot(
+    mod_spec |>
+      fit(mpg ~ ., mtcars)
+  )
+})
+
 test_that("training wrapper warns on protected arguments", {
   skip_if_not_installed("lightgbm")
   skip_if_not_installed("modeldata")