From 5b833aa0a90c03fa805fb656855e9bc98374fff7 Mon Sep 17 00:00:00 2001 From: david-cortes Date: Thu, 2 Nov 2023 13:36:03 +0100 Subject: [PATCH] [R-package] Use `cat()` instead of `print()` for metrics and callbacks (#6171) --- .ci/lint_r_code.R | 3 +-- R-package/R/callback.R | 10 +++++----- R-package/tests/testthat/test_basic.R | 23 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/.ci/lint_r_code.R b/.ci/lint_r_code.R index a49e70f9492b..d5b1217a5b04 100755 --- a/.ci/lint_r_code.R +++ b/.ci/lint_r_code.R @@ -78,8 +78,7 @@ LINTERS_TO_USE <- list( , "true_false" = lintr::T_and_F_symbol_linter() , "undesirable_function" = lintr::undesirable_function_linter( fun = c( - "cat" = "CRAN forbids the use of cat() in packages except in special cases. Use message() or warning()." - , "cbind" = paste0( + "cbind" = paste0( "cbind is an unsafe way to build up a data frame. merge() or direct " , "column assignment is preferred." ) diff --git a/R-package/R/callback.R b/R-package/R/callback.R index e428dfb79eea..3569b47f5b14 100644 --- a/R-package/R/callback.R +++ b/R-package/R/callback.R @@ -90,7 +90,7 @@ cb_print_evaluation <- function(period) { # Check if message is existing if (nchar(msg) > 0L) { - print(.merge_eval_string(env = env)) + cat(.merge_eval_string(env = env), "\n") } } @@ -208,9 +208,9 @@ cb_early_stop <- function(stopping_rounds, first_metric_only, verbose) { msg <- paste0( "Will train until there is no improvement in " , stopping_rounds - , " rounds." + , " rounds.\n" ) - print(msg) + cat(msg) } # Internally treat everything as a maximization task @@ -284,7 +284,7 @@ cb_early_stop <- function(stopping_rounds, first_metric_only, verbose) { } if (isTRUE(verbose)) { - print(paste0("Early stopping, best iteration is: ", best_msg[[i]])) + cat(paste0("Early stopping, best iteration is: ", best_msg[[i]], "\n")) } # Store best iteration and stop @@ -302,7 +302,7 @@ cb_early_stop <- function(stopping_rounds, first_metric_only, verbose) { } if (isTRUE(verbose)) { - print(paste0("Did not meet early stopping, best iteration is: ", best_msg[[i]])) + cat(paste0("Did not meet early stopping, best iteration is: ", best_msg[[i]], "\n")) } # Store best iteration and stop diff --git a/R-package/tests/testthat/test_basic.R b/R-package/tests/testthat/test_basic.R index 80e8b55feb75..c58ea9b53e51 100644 --- a/R-package/tests/testthat/test_basic.R +++ b/R-package/tests/testthat/test_basic.R @@ -3810,6 +3810,29 @@ test_that("lightgbm() correctly sets objective when passing lgb.Dataset as input expect_equal(model$params$objective, "regression") }) +test_that("Evaluation metrics aren't printed as a single-element vector", { + log_txt <- capture_output({ + data(mtcars) + y <- mtcars$mpg + x <- as.matrix(mtcars[, -1L]) + cv_result <- lgb.cv( + data = lgb.Dataset(x, label = y) + , params = list( + objective = "regression" + , metric = "l2" + , min_data_in_leaf = 5L + , max_depth = 3L + , num_threads = .LGB_MAX_THREADS + ) + , nrounds = 2L + , nfold = 3L + , verbose = 1L + , eval_train_metric = TRUE + ) + }) + expect_false(grepl("[1] \"[1]", log_txt, fixed = TRUE)) +}) + test_that("lgb.cv() doesn't mix booster messages with evaluation metrics messages", { log_txt <- capture_output({ data(mtcars)