Apply suggestions from code review

Co-authored-by: James Lamb <jaylamb20@gmail.com>
microsoft · Aug 19, 2024 · d38b84c · d38b84c
1 parent a2d7928
commit d38b84c
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 22 deletions.
diff --git a/R-package/R/lgb.cv.R b/R-package/R/lgb.cv.R
@@ -295,9 +295,9 @@ lgb.cv <- function(params = list()
 
   # Cannot use early stopping with 'dart' boosting
   if (using_dart) {
-    if (using_early_stopping)
+    if (using_early_stopping){
       warning("Early stopping is not available in 'dart' mode.")
-
+    }
     using_early_stopping <- FALSE
 
     # Remove the cb_early_stop() function if it was passed in to callbacks

diff --git a/R-package/tests/testthat/test_parameters.R b/R-package/tests/testthat/test_parameters.R
@@ -91,7 +91,7 @@ test_that(".PARAMETER_ALIASES() uses the internal session cache", {
   expect_false(exists(cache_key, where = .lgb_session_cache_env))
 })
 
-test_that("training should only warn if you use 'dart' boosting, specified with 'boosting' or aliases", {
+test_that("training should warn if you use 'dart' boosting and requesting early stopping, specified with 'boosting' or aliases", {
   for (boosting_param in .PARAMETER_ALIASES()[["boosting"]]) {
     params <- list(
         num_leaves = 5L
@@ -102,8 +102,7 @@ test_that("training should only warn if you use 'dart' boosting, specified with
     )
     params[[boosting_param]] <- "dart"
 
-    # expect warning
-    # if early_stopping_rounds is specified
+    # warning: early stopping requested
     expect_warning({
       result <- lightgbm(
         data = train$data
@@ -115,10 +114,7 @@ test_that("training should only warn if you use 'dart' boosting, specified with
       )
     }, regexp = "Early stopping is not available in 'dart' mode")
 
-    # expect no warning
-    # if early_stopping_rounds is not
-    # specified see:
-    # https://github.com/microsoft/LightGBM/issues/6612
+    # no warning: early stopping not requested
     expect_no_warning({
       result <- lightgbm(
         data = train$data
@@ -139,20 +135,18 @@ test_that(
     for (boosting_param in .PARAMETER_ALIASES()[["boosting"]]) {
       params <- list(
         num_leaves = 5L
-        , learning_rate = 0.05
         , objective = "binary"
         , metric = "binary_error"
         , num_threads = .LGB_MAX_THREADS
       )
       params[[boosting_param]] <- "dart"
 
-      # expect warning
-      # if early_stopping_rounds is specified
+      # warning: early stopping requested
       expect_warning({
         result <- lgb.cv(
           data = lgb.Dataset(
-            data  = train$data,
-            label = train$label
+            data  = train$data
+            , label = train$label
           )
           , params = params
           , nrounds = 5L
@@ -161,23 +155,19 @@ test_that(
         )
       }, regexp = "Early stopping is not available in 'dart' mode")
 
-      # expect no warning
-      # if early_stopping_rounds is not
-      # specified see:
-      # https://github.com/microsoft/LightGBM/issues/6612
+      # no warning: early stopping not requested
       expect_no_warning({
         result <- lgb.cv(
           data = lgb.Dataset(
-            data  = train$data,
-            label = train$label
+            data  = train$data
+            , label = train$label
           )
           , params = params
           , nrounds = 5L,
           , verbose = -1L
           , early_stopping_rounds = NULL
         )
-      }
-      )
+      })
     }
   }
 )