Skip to content

Commit

Permalink
[R-package] fix R CMD check NOTE about leftover files (#3181)
Browse files Browse the repository at this point in the history
* [R-package] fix R CMD check NOTE about leftover files

* update number of allowed notes
  • Loading branch information
jameslamb authored Jun 23, 2020
1 parent bca2da9 commit be85f56
Show file tree
Hide file tree
Showing 15 changed files with 65 additions and 33 deletions.
6 changes: 3 additions & 3 deletions .ci/test_r_package_windows.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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"
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions R-package/R/lgb.Booster.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
#' }
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 7 additions & 5 deletions R-package/R/lgb.Dataset.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {

Expand Down
5 changes: 3 additions & 2 deletions R-package/R/readRDS.lgb.Booster.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion R-package/R/saveRDS.lgb.Booster.R
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 3 additions & 2 deletions R-package/man/lgb.Dataset.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion R-package/man/lgb.Dataset.save.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions R-package/man/lgb.Dataset.set.categorical.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions R-package/man/lgb.load.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion R-package/man/lgb.save.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions R-package/man/readRDS.lgb.Booster.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion R-package/man/saveRDS.lgb.Booster.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions R-package/tests/testthat/test_basic.R
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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))
Expand All @@ -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(
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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")
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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"
Expand Down
23 changes: 17 additions & 6 deletions R-package/tests/testthat/test_lgb.Booster.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)) {
Expand Down Expand Up @@ -147,19 +148,21 @@ 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()
expect_null(bst$.__enclos_env__$private$handle)
rm(bst)

bst2 <- lgb.load(
filename = "lightgbm.model"
filename = model_file
)
pred2 <- predict(bst2, test$data)
expect_identical(pred, pred2)
Expand All @@ -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))

Expand Down Expand Up @@ -209,19 +213,21 @@ 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()
expect_null(bst$.__enclos_env__$private$handle)
rm(bst)

bst2 <- lgb.load(
filename = "lightgbm.model"
filename = model_file
, model_str = 4.0
)
pred2 <- predict(bst2, test$data)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions R-package/tests/testthat/test_parameters.R
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
})

Expand Down Expand Up @@ -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")
}
Expand Down

0 comments on commit be85f56

Please sign in to comment.