From 441f38bb8140cab9ea3076f903e66541a5c48785 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Sat, 5 Jun 2021 13:51:48 +0100 Subject: [PATCH] [R-package] remove lgb.last_error() and LGBM_GetLastError_R() (#4344) --- R-package/R/utils.R | 10 ---------- R-package/src/lightgbm_R.cpp | 9 --------- R-package/src/lightgbm_R.h | 6 ------ R-package/tests/testthat/test_utils.R | 24 ------------------------ build_r.R | 2 +- 5 files changed, 1 insertion(+), 50 deletions(-) diff --git a/R-package/R/utils.R b/R-package/R/utils.R index 313bb9bf57be..d69a8d1b735b 100644 --- a/R-package/R/utils.R +++ b/R-package/R/utils.R @@ -15,16 +15,6 @@ lgb.is.null.handle <- function(x) { ) } -# [description] Get the most recent error stored on the C++ side and raise it -# as an R error. -lgb.last_error <- function() { - err_msg <- .Call( - LGBM_GetLastError_R - ) - stop("api error: ", err_msg) - return(invisible(NULL)) -} - lgb.params2str <- function(params) { # Check for a list as input diff --git a/R-package/src/lightgbm_R.cpp b/R-package/src/lightgbm_R.cpp index 44a37ae3206a..74704d92b1ec 100644 --- a/R-package/src/lightgbm_R.cpp +++ b/R-package/src/lightgbm_R.cpp @@ -42,14 +42,6 @@ using LightGBM::Common::Split; using LightGBM::Log; -SEXP LGBM_GetLastError_R() { - SEXP out; - out = PROTECT(Rf_allocVector(STRSXP, 1)); - SET_STRING_ELT(out, 0, Rf_mkChar(LGBM_GetLastError())); - UNPROTECT(1); - return out; -} - SEXP LGBM_HandleIsNull_R(SEXP handle) { return Rf_ScalarLogical(R_ExternalPtrAddr(handle) == NULL); } @@ -712,7 +704,6 @@ SEXP LGBM_BoosterDumpModel_R(SEXP handle, // .Call() calls static const R_CallMethodDef CallEntries[] = { - {"LGBM_GetLastError_R" , (DL_FUNC) &LGBM_GetLastError_R , 0}, {"LGBM_HandleIsNull_R" , (DL_FUNC) &LGBM_HandleIsNull_R , 1}, {"LGBM_DatasetCreateFromFile_R" , (DL_FUNC) &LGBM_DatasetCreateFromFile_R , 3}, {"LGBM_DatasetCreateFromCSC_R" , (DL_FUNC) &LGBM_DatasetCreateFromCSC_R , 8}, diff --git a/R-package/src/lightgbm_R.h b/R-package/src/lightgbm_R.h index 588d6aa37218..16a87c3a611f 100644 --- a/R-package/src/lightgbm_R.h +++ b/R-package/src/lightgbm_R.h @@ -11,12 +11,6 @@ #define R_USE_C99_IN_CXX #include -/*! -* \brief get string message of the last error -* \return err_msg string with error information -*/ -LIGHTGBM_C_EXPORT SEXP LGBM_GetLastError_R(); - /*! * \brief check if an R external pointer (like a Booster or Dataset handle) is a null pointer * \param handle handle for a Booster, Dataset, or Predictor diff --git a/R-package/tests/testthat/test_utils.R b/R-package/tests/testthat/test_utils.R index 491ded72d994..24f20520c2ed 100644 --- a/R-package/tests/testthat/test_utils.R +++ b/R-package/tests/testthat/test_utils.R @@ -48,30 +48,6 @@ test_that("lgb.params2str() works as expected for a key in params with multiple ) }) -context("lgb.last_error") - -test_that("lgb.last_error() throws an error if there are no errors", { - expect_error({ - lgb.last_error() - }, regexp = "Everything is fine") -}) - -test_that("lgb.last_error() correctly returns errors from the C++ side", { - testthat::skip(paste0( - "Skipping this test because it causes valgrind to think " - , "there is a memory leak, and needs to be rethought" - )) - data(agaricus.train, package = "lightgbm") - train <- agaricus.train - dvalid1 <- lgb.Dataset( - data = train$data - , label = as.matrix(rnorm(5L)) - ) - expect_error({ - dvalid1$construct() - }, regexp = "[LightGBM] [Fatal] Length of label is not same with #data", fixed = TRUE) -}) - context("lgb.check.eval") test_that("lgb.check.eval works as expected with no metric", { diff --git a/build_r.R b/build_r.R index d25fe28350a3..aa16c6fcdc6c 100644 --- a/build_r.R +++ b/build_r.R @@ -372,7 +372,7 @@ writeLines(description_contents, DESCRIPTION_FILE) # CMake-based builds can't currently use R's builtin routine registration, # so have to update NAMESPACE manually, with a statement like this: # -# useDynLib(lib_lightgbm, LGBM_GetLastError_R, LGBM_DatasetCreateFromFile_R, ...) +# useDynLib(lib_lightgbm, LGBM_DatasetCreateFromFile_R, ...) # # See https://cran.r-project.org/doc/manuals/r-release/R-exts.html#useDynLib for # documentation of this approach, where the NAMESPACE file uses a statement like