From be85f56ffc0d3939a7140fc8686c7ba695aefdb5 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Tue, 23 Jun 2020 09:01:02 -0500 Subject: [PATCH] [R-package] fix R CMD check NOTE about leftover files (#3181) * [R-package] fix R CMD check NOTE about leftover files * update number of allowed notes --- .ci/test_r_package_windows.ps1 | 6 ++--- R-package/R/lgb.Booster.R | 7 +++--- R-package/R/lgb.Dataset.R | 12 +++++----- R-package/R/readRDS.lgb.Booster.R | 5 +++-- R-package/R/saveRDS.lgb.Booster.R | 3 ++- R-package/man/lgb.Dataset.Rd | 5 +++-- R-package/man/lgb.Dataset.save.Rd | 2 +- R-package/man/lgb.Dataset.set.categorical.Rd | 5 +++-- R-package/man/lgb.load.Rd | 5 +++-- R-package/man/lgb.save.Rd | 2 +- R-package/man/readRDS.lgb.Booster.Rd | 5 +++-- R-package/man/saveRDS.lgb.Booster.Rd | 3 ++- R-package/tests/testthat/test_basic.R | 13 +++++++++-- R-package/tests/testthat/test_lgb.Booster.R | 23 +++++++++++++++----- R-package/tests/testthat/test_parameters.R | 2 ++ 15 files changed, 65 insertions(+), 33 deletions(-) diff --git a/.ci/test_r_package_windows.ps1 b/.ci/test_r_package_windows.ps1 index e87c1118d555..c13ca29f399f 100644 --- a/.ci/test_r_package_windows.ps1 +++ b/.ci/test_r_package_windows.ps1 @@ -124,7 +124,7 @@ if ($env:COMPILER -ne "MSVC") { .\miktex\download\miktexsetup.exe --remote-package-repository="$env:CTAN_PACKAGE_ARCHIVE" --portable="$env:R_LIB_PATH/miktex" --quiet install ; Check-Output $? Write-Output "Done installing MiKTeX" - Run-R-Code-Redirect-Stderr "processx::run(command = 'initexmf', args = c('--set-config-value', '[MPM]AutoInstall=1'), windows_verbatim_args = TRUE)" ; Check-Output $? + Run-R-Code-Redirect-Stderr "result <- processx::run(command = 'initexmf', args = c('--set-config-value', '[MPM]AutoInstall=1'), echo = TRUE, windows_verbatim_args = TRUE)" ; Check-Output $? conda install -q -y --no-deps pandoc } @@ -140,7 +140,7 @@ if ($env:COMPILER -ne "MSVC") { $env:_R_CHECK_FORCE_SUGGESTS_ = 0 Write-Output "Running R CMD check as CRAN" - Run-R-Code-Redirect-Stderr "processx::run(command = 'R.exe', args = c('CMD', 'check', '--no-multiarch', '--as-cran', '$PKG_FILE_NAME'), windows_verbatim_args = FALSE)" ; $check_succeeded = $? + Run-R-Code-Redirect-Stderr "result <- processx::run(command = 'R.exe', args = c('CMD', 'check', '--no-multiarch', '--as-cran', '$PKG_FILE_NAME'), echo = TRUE, windows_verbatim_args = FALSE)" ; $check_succeeded = $? Write-Output "R CMD check build logs:" $INSTALL_LOG_FILE_NAME = "$env:BUILD_SOURCESDIRECTORY\lightgbm.Rcheck\00install.out" @@ -157,7 +157,7 @@ if ($env:COMPILER -ne "MSVC") { $note_str = Get-Content -Path "${LOG_FILE_NAME}" | Select-String -Pattern '.*Status.* NOTE' | Out-String ; Check-Output $? $relevant_line = $note_str -match '(\d+) NOTE' $NUM_CHECK_NOTES = $matches[1] - $ALLOWED_CHECK_NOTES = 4 + $ALLOWED_CHECK_NOTES = 3 if ([int]$NUM_CHECK_NOTES -gt $ALLOWED_CHECK_NOTES) { Write-Output "Found ${NUM_CHECK_NOTES} NOTEs from R CMD check. Only ${ALLOWED_CHECK_NOTES} are allowed" Check-Output $False diff --git a/R-package/R/lgb.Booster.R b/R-package/R/lgb.Booster.R index 101cad0b998e..722783e4be02 100644 --- a/R-package/R/lgb.Booster.R +++ b/R-package/R/lgb.Booster.R @@ -789,8 +789,9 @@ predict.lgb.Booster <- function(object, #' , learning_rate = 1.0 #' , early_stopping_rounds = 3L #' ) -#' lgb.save(model, "model.txt") -#' load_booster <- lgb.load(filename = "model.txt") +#' model_file <- tempfile(fileext = ".txt") +#' lgb.save(model, model_file) +#' load_booster <- lgb.load(filename = model_file) #' model_string <- model$save_model_to_string(NULL) # saves best iteration #' load_booster_from_str <- lgb.load(model_str = model_string) #' } @@ -849,7 +850,7 @@ lgb.load <- function(filename = NULL, model_str = NULL) { #' , learning_rate = 1.0 #' , early_stopping_rounds = 5L #' ) -#' lgb.save(model, "lgb-model.txt") +#' lgb.save(model, tempfile(fileext = ".txt")) #' } #' @export lgb.save <- function(booster, filename, num_iteration = NULL) { diff --git a/R-package/R/lgb.Dataset.R b/R-package/R/lgb.Dataset.R index fed95913d2d6..88cc60bd6a96 100644 --- a/R-package/R/lgb.Dataset.R +++ b/R-package/R/lgb.Dataset.R @@ -728,8 +728,9 @@ Dataset <- R6::R6Class( #' data(agaricus.train, package = "lightgbm") #' train <- agaricus.train #' dtrain <- lgb.Dataset(train$data, label = train$label) -#' lgb.Dataset.save(dtrain, "lgb.Dataset.data") -#' dtrain <- lgb.Dataset("lgb.Dataset.data") +#' data_file <- tempfile(fileext = ".data") +#' lgb.Dataset.save(dtrain, data_file) +#' dtrain <- lgb.Dataset(data_file) #' lgb.Dataset.construct(dtrain) #' #' @export @@ -1073,8 +1074,9 @@ setinfo.lgb.Dataset <- function(dataset, name, info, ...) { #' data(agaricus.train, package = "lightgbm") #' train <- agaricus.train #' dtrain <- lgb.Dataset(train$data, label = train$label) -#' lgb.Dataset.save(dtrain, "lgb-Dataset.data") -#' dtrain <- lgb.Dataset("lgb-Dataset.data") +#' data_file <- tempfile(fileext = ".data") +#' lgb.Dataset.save(dtrain, data_file) +#' dtrain <- lgb.Dataset(data_file) #' lgb.Dataset.set.categorical(dtrain, 1L:2L) #' #' @rdname lgb.Dataset.set.categorical @@ -1134,7 +1136,7 @@ lgb.Dataset.set.reference <- function(dataset, reference) { #' data(agaricus.train, package = "lightgbm") #' train <- agaricus.train #' dtrain <- lgb.Dataset(train$data, label = train$label) -#' lgb.Dataset.save(dtrain, "data.bin") +#' lgb.Dataset.save(dtrain, tempfile(fileext = ".bin")) #' @export lgb.Dataset.save <- function(dataset, fname) { diff --git a/R-package/R/readRDS.lgb.Booster.R b/R-package/R/readRDS.lgb.Booster.R index f0c862f33c74..23c1afbee4d4 100644 --- a/R-package/R/readRDS.lgb.Booster.R +++ b/R-package/R/readRDS.lgb.Booster.R @@ -26,8 +26,9 @@ #' , learning_rate = 1.0 #' , early_stopping_rounds = 5L #' ) -#' saveRDS.lgb.Booster(model, "model.rds") -#' new_model <- readRDS.lgb.Booster("model.rds") +#' model_file <- tempfile(fileext = ".rds") +#' saveRDS.lgb.Booster(model, model_file) +#' new_model <- readRDS.lgb.Booster(model_file) #' } #' @export readRDS.lgb.Booster <- function(file = "", refhook = NULL) { diff --git a/R-package/R/saveRDS.lgb.Booster.R b/R-package/R/saveRDS.lgb.Booster.R index 185186c9bc25..7a2f838e9ff5 100644 --- a/R-package/R/saveRDS.lgb.Booster.R +++ b/R-package/R/saveRDS.lgb.Booster.R @@ -37,7 +37,8 @@ #' , learning_rate = 1.0 #' , early_stopping_rounds = 5L #' ) -#' saveRDS.lgb.Booster(model, "lgb-model.rds") +#' model_file <- tempfile(fileext = ".rds") +#' saveRDS.lgb.Booster(model, model_file) #' } #' @export saveRDS.lgb.Booster <- function(object, diff --git a/R-package/man/lgb.Dataset.Rd b/R-package/man/lgb.Dataset.Rd index fb1d1067a53e..52e9b74058ee 100644 --- a/R-package/man/lgb.Dataset.Rd +++ b/R-package/man/lgb.Dataset.Rd @@ -43,8 +43,9 @@ Construct \code{lgb.Dataset} object from dense matrix, sparse matrix data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) -lgb.Dataset.save(dtrain, "lgb.Dataset.data") -dtrain <- lgb.Dataset("lgb.Dataset.data") +data_file <- tempfile(fileext = ".data") +lgb.Dataset.save(dtrain, data_file) +dtrain <- lgb.Dataset(data_file) lgb.Dataset.construct(dtrain) } diff --git a/R-package/man/lgb.Dataset.save.Rd b/R-package/man/lgb.Dataset.save.Rd index 26895999d11a..699788d39977 100644 --- a/R-package/man/lgb.Dataset.save.Rd +++ b/R-package/man/lgb.Dataset.save.Rd @@ -22,5 +22,5 @@ Please note that \code{init_score} is not saved in binary file. data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) -lgb.Dataset.save(dtrain, "data.bin") +lgb.Dataset.save(dtrain, tempfile(fileext = ".bin")) } diff --git a/R-package/man/lgb.Dataset.set.categorical.Rd b/R-package/man/lgb.Dataset.set.categorical.Rd index 097eea02b465..debe7e02d151 100644 --- a/R-package/man/lgb.Dataset.set.categorical.Rd +++ b/R-package/man/lgb.Dataset.set.categorical.Rd @@ -24,8 +24,9 @@ Set the categorical features of an \code{lgb.Dataset} object. Use this function data(agaricus.train, package = "lightgbm") train <- agaricus.train dtrain <- lgb.Dataset(train$data, label = train$label) -lgb.Dataset.save(dtrain, "lgb-Dataset.data") -dtrain <- lgb.Dataset("lgb-Dataset.data") +data_file <- tempfile(fileext = ".data") +lgb.Dataset.save(dtrain, data_file) +dtrain <- lgb.Dataset(data_file) lgb.Dataset.set.categorical(dtrain, 1L:2L) } diff --git a/R-package/man/lgb.load.Rd b/R-package/man/lgb.load.Rd index 5f7c2354733e..72633e7baef8 100644 --- a/R-package/man/lgb.load.Rd +++ b/R-package/man/lgb.load.Rd @@ -37,8 +37,9 @@ model <- lgb.train( , learning_rate = 1.0 , early_stopping_rounds = 3L ) -lgb.save(model, "model.txt") -load_booster <- lgb.load(filename = "model.txt") +model_file <- tempfile(fileext = ".txt") +lgb.save(model, model_file) +load_booster <- lgb.load(filename = model_file) model_string <- model$save_model_to_string(NULL) # saves best iteration load_booster_from_str <- lgb.load(model_str = model_string) } diff --git a/R-package/man/lgb.save.Rd b/R-package/man/lgb.save.Rd index f1ffd48355ee..9ac19eadb3fc 100644 --- a/R-package/man/lgb.save.Rd +++ b/R-package/man/lgb.save.Rd @@ -39,6 +39,6 @@ model <- lgb.train( , learning_rate = 1.0 , early_stopping_rounds = 5L ) -lgb.save(model, "lgb-model.txt") +lgb.save(model, tempfile(fileext = ".txt")) } } diff --git a/R-package/man/readRDS.lgb.Booster.Rd b/R-package/man/readRDS.lgb.Booster.Rd index be03fd1cfcb8..35b4ff6ea5f3 100644 --- a/R-package/man/readRDS.lgb.Booster.Rd +++ b/R-package/man/readRDS.lgb.Booster.Rd @@ -37,7 +37,8 @@ model <- lgb.train( , learning_rate = 1.0 , early_stopping_rounds = 5L ) -saveRDS.lgb.Booster(model, "model.rds") -new_model <- readRDS.lgb.Booster("model.rds") +model_file <- tempfile(fileext = ".rds") +saveRDS.lgb.Booster(model, model_file) +new_model <- readRDS.lgb.Booster(model_file) } } diff --git a/R-package/man/saveRDS.lgb.Booster.Rd b/R-package/man/saveRDS.lgb.Booster.Rd index 66afa861db9f..f267293f9d96 100644 --- a/R-package/man/saveRDS.lgb.Booster.Rd +++ b/R-package/man/saveRDS.lgb.Booster.Rd @@ -61,6 +61,7 @@ model <- lgb.train( , learning_rate = 1.0 , early_stopping_rounds = 5L ) -saveRDS.lgb.Booster(model, "lgb-model.rds") +model_file <- tempfile(fileext = ".rds") +saveRDS.lgb.Booster(model, model_file) } } diff --git a/R-package/tests/testthat/test_basic.R b/R-package/tests/testthat/test_basic.R index 1b22a894a04e..c7583ac77154 100644 --- a/R-package/tests/testthat/test_basic.R +++ b/R-package/tests/testthat/test_basic.R @@ -16,6 +16,7 @@ test_that("train and predict binary classification", { , nrounds = nrounds , objective = "binary" , metric = "binary_error" + , save_name = tempfile(fileext = ".model") ) expect_false(is.null(bst$record_evals)) record_results <- lgb.get.eval.result(bst, "train", "binary_error") @@ -47,6 +48,7 @@ test_that("train and predict softmax", { , objective = "multiclass" , metric = "multi_error" , num_class = 3L + , save_name = tempfile(fileext = ".model") ) expect_false(is.null(bst$record_evals)) @@ -68,6 +70,7 @@ test_that("use of multiple eval metrics works", { , nrounds = 10L , objective = "binary" , metric = metrics + , save_name = tempfile(fileext = ".model") ) expect_false(is.null(bst$record_evals)) expect_named( @@ -88,6 +91,7 @@ test_that("lgb.Booster.upper_bound() and lgb.Booster.lower_bound() work as expec , nrounds = nrounds , objective = "binary" , metric = "binary_error" + , save_name = tempfile(fileext = ".model") ) expect_true(abs(bst$lower_bound() - -1.590853) < TOLERANCE) expect_true(abs(bst$upper_bound() - 1.871015) < TOLERANCE) @@ -103,6 +107,7 @@ test_that("lgb.Booster.upper_bound() and lgb.Booster.lower_bound() work as expec , nrounds = nrounds , objective = "regression" , metric = "l2" + , save_name = tempfile(fileext = ".model") ) expect_true(abs(bst$lower_bound() - 0.1513859) < TOLERANCE) expect_true(abs(bst$upper_bound() - 0.9080349) < TOLERANCE) @@ -117,6 +122,7 @@ test_that("lightgbm() rejects negative or 0 value passed to nrounds", { data = dtrain , params = params , nrounds = nround_value + , save_name = tempfile(fileext = ".model") ) }, "nrounds should be greater than zero") } @@ -147,6 +153,7 @@ test_that("lightgbm() performs evaluation on validation sets if they are provide "valid1" = dvalid1 , "valid2" = dvalid2 ) + , save_name = tempfile(fileext = ".model") ) expect_named( @@ -188,13 +195,14 @@ test_that("training continuation works", { # first 5 iterations: bst1 <- lgb.train(param, dtrain, nrounds = 5L, watchlist) # test continuing from a model in file - lgb.save(bst1, "lightgbm.model") + model_file <- tempfile(fileext = ".model") + lgb.save(bst1, model_file) # continue for 5 more: bst2 <- lgb.train(param, dtrain, nrounds = 5L, watchlist, init_model = bst1) err_bst2 <- lgb.get.eval.result(bst2, "train", "binary_logloss", 10L) expect_lt(abs(err_bst - err_bst2), 0.01) - bst2 <- lgb.train(param, dtrain, nrounds = 5L, watchlist, init_model = "lightgbm.model") + bst2 <- lgb.train(param, dtrain, nrounds = 5L, watchlist, init_model = model_file) err_bst2 <- lgb.get.eval.result(bst2, "train", "binary_logloss", 10L) expect_lt(abs(err_bst - err_bst2), 0.01) }) @@ -1007,6 +1015,7 @@ test_that("using lightgbm() without early stopping, best_iter and best_score com , learning_rate = 1.5 ) , verbose = -7L + , save_name = tempfile(fileext = ".model") ) # when verbose <= 0 is passed to lightgbm(), 'valids' is passed through to lgb.train() # untouched. If you set verbose to > 0, the training data will still be first but called "train" diff --git a/R-package/tests/testthat/test_lgb.Booster.R b/R-package/tests/testthat/test_lgb.Booster.R index f418a96d36bf..12a12a0e7703 100644 --- a/R-package/tests/testthat/test_lgb.Booster.R +++ b/R-package/tests/testthat/test_lgb.Booster.R @@ -104,6 +104,7 @@ test_that("lgb.load() gives the expected error messages given different incorrec , learning_rate = 1.0 , nrounds = 2L , objective = "binary" + , save_name = tempfile(fileext = ".model") ) # you have to give model_str or filename @@ -115,9 +116,9 @@ test_that("lgb.load() gives the expected error messages given different incorrec }, regexp = "either filename or model_str must be given") # if given, filename should be a string that points to an existing file - out_file <- "lightgbm.model" + model_file <- tempfile(fileext = ".model") expect_error({ - lgb.load(filename = list(out_file)) + lgb.load(filename = list(model_file)) }, regexp = "filename should be character") file_to_check <- paste0("a.model") while (file.exists(file_to_check)) { @@ -147,11 +148,13 @@ test_that("Loading a Booster from a file works", { , learning_rate = 1.0 , nrounds = 2L , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_true(lgb.is.Booster(bst)) pred <- predict(bst, test$data) - lgb.save(bst, "lightgbm.model") + model_file <- tempfile(fileext = ".model") + lgb.save(bst, model_file) # finalize the booster and destroy it so you know we aren't cheating bst$finalize() @@ -159,7 +162,7 @@ test_that("Loading a Booster from a file works", { rm(bst) bst2 <- lgb.load( - filename = "lightgbm.model" + filename = model_file ) pred2 <- predict(bst2, test$data) expect_identical(pred, pred2) @@ -178,6 +181,7 @@ test_that("Loading a Booster from a string works", { , learning_rate = 1.0 , nrounds = 2L , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_true(lgb.is.Booster(bst)) @@ -209,11 +213,13 @@ test_that("If a string and a file are both passed to lgb.load() the file is used , learning_rate = 1.0 , nrounds = 2L , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_true(lgb.is.Booster(bst)) pred <- predict(bst, test$data) - lgb.save(bst, "lightgbm.model") + model_file <- tempfile(fileext = ".model") + lgb.save(bst, model_file) # finalize the booster and destroy it so you know we aren't cheating bst$finalize() @@ -221,7 +227,7 @@ test_that("If a string and a file are both passed to lgb.load() the file is used rm(bst) bst2 <- lgb.load( - filename = "lightgbm.model" + filename = model_file , model_str = 4.0 ) pred2 <- predict(bst2, test$data) @@ -261,6 +267,7 @@ test_that("Creating a Booster from a Dataset with an existing predictor should w , learning_rate = 1.0 , nrounds = nrounds , objective = "binary" + , save_name = tempfile(fileext = ".model") ) data(agaricus.test, package = "lightgbm") dtest <- Dataset$new( @@ -294,6 +301,7 @@ test_that("Booster$rollback_one_iter() should work as expected", { , learning_rate = 1.0 , nrounds = nrounds , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_equal(bst$current_iter(), nrounds) expect_true(lgb.is.Booster(bst)) @@ -325,6 +333,7 @@ test_that("Booster$update() passing a train_set works as expected", { , learning_rate = 1.0 , nrounds = nrounds , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_true(lgb.is.Booster(bst)) expect_equal(bst$current_iter(), nrounds) @@ -345,6 +354,7 @@ test_that("Booster$update() passing a train_set works as expected", { , learning_rate = 1.0 , nrounds = nrounds + 1L , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_true(lgb.is.Booster(bst2)) expect_equal(bst2$current_iter(), nrounds + 1L) @@ -367,6 +377,7 @@ test_that("Booster$update() throws an informative error if you provide a non-Dat , learning_rate = 1.0 , nrounds = nrounds , objective = "binary" + , save_name = tempfile(fileext = ".model") ) expect_error({ bst$update( diff --git a/R-package/tests/testthat/test_parameters.R b/R-package/tests/testthat/test_parameters.R index 9035b7e0a6ca..16d1e4a5a5e2 100644 --- a/R-package/tests/testthat/test_parameters.R +++ b/R-package/tests/testthat/test_parameters.R @@ -24,6 +24,7 @@ test_that("Feature penalties work properly", { , feature_penalty = paste0(feature_penalties, collapse = ",") , metric = "binary_error" , verbose = -1L + , save_name = tempfile(fileext = ".model") ) }) @@ -75,6 +76,7 @@ test_that("training should warn if you use 'dart' boosting, specified with 'boos object = "dart" , nm = boosting_param ) + , save_name = tempfile(fileext = ".model") ) }, regexp = "Early stopping is not available in 'dart' mode") }