From 5705ac640b36d94650a9ace33b3fd6495bd70afa Mon Sep 17 00:00:00 2001 From: Sanidhya Vijay Date: Tue, 19 May 2026 15:11:34 +0530 Subject: [PATCH 1/2] Fixed silent ignore of legacy predict flags (#12211) predict.xgboost was silently ignoring boolean flags like predcontrib=TRUE since they were absorbed by '...'. This maps those legacy flags to the appropriate 'type' and issues a deprecation warning so we don't break backward compatibility. Any completely unknown arguments in '...' will now throw a proper error. --- R-package/R/xgboost.R | 34 +++++++++++++++++++++++++ R-package/tests/testthat/test_xgboost.R | 22 ++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/R-package/R/xgboost.R b/R-package/R/xgboost.R index b6f9c8bc5518..9bb1ebd1636c 100644 --- a/R-package/R/xgboost.R +++ b/R-package/R/xgboost.R @@ -1352,6 +1352,40 @@ predict.xgboost <- function( validate_features = TRUE, ... ) { + dots <- list(...) + if (length(dots) > 0) { + mapping <- list( + outputmargin = "raw", + predleaf = "leaf", + predcontrib = "contrib", + approxcontrib = "contrib", + predinteraction = "interaction" + ) + found_legacy <- intersect(names(dots), names(mapping)) + + if (length(found_legacy) > 0) { + legacy_arg <- found_legacy[1] + if (isTRUE(dots[[legacy_arg]])) { + type <- mapping[[legacy_arg]] + warning( + sprintf( + "Argument '%s' is deprecated. Please use type = '%s' instead.", + legacy_arg, type + ), + call. = FALSE + ) + } + dots <- dots[setdiff(names(dots), names(mapping))] + } + + if (length(dots) > 0) { + stop( + "predict.xgboost: arguments in '...' are not supported (", + paste(names(dots), collapse = ", "), ")." + ) + } + } + if (inherits(newdata, "xgb.DMatrix")) { stop( "Predictions on 'xgb.DMatrix' objects are not supported with 'xgboost' class.", diff --git a/R-package/tests/testthat/test_xgboost.R b/R-package/tests/testthat/test_xgboost.R index a8e3537d2e6e..e70077092058 100644 --- a/R-package/tests/testthat/test_xgboost.R +++ b/R-package/tests/testthat/test_xgboost.R @@ -1131,3 +1131,25 @@ test_that("Linear booster importance uses class names", { expect_true(is.factor(imp$Class)) expect_equal(levels(imp$Class), levels(y)) }) + +test_that("predict.xgboost maps legacy boolean flags to type", { + y <- mtcars$mpg + x <- as.matrix(mtcars[, -1L]) + model <- xgboost(x, y, nthreads = 1L, nrounds = 1L) + + expect_warning( + pred <- predict(model, x, predcontrib = TRUE), + paste0( + "Argument 'predcontrib' is deprecated. ", + "Please use type = 'contrib' instead." + ), + fixed = TRUE + ) + expect_equal(dim(pred), c(nrow(x), ncol(x) + 1L)) + + expect_error( + predict(model, x, foobar = TRUE), + "predict.xgboost: arguments in '...' are not supported (foobar).", + fixed = TRUE + ) +}) From 4ffd8fd60c8498e6f1fdeb0410c4f7efaf95efbc Mon Sep 17 00:00:00 2001 From: Sanidhya Vijay Date: Tue, 19 May 2026 16:18:05 +0530 Subject: [PATCH 2/2] Refined legacy flag mapping and improve error handling - Improved intent detection by checking for isTRUE() in legacy flags. - Use dots[found_legacy] <- NULL for safer subsetting. - Handle unnamed arguments in '...' with descriptive '' error messages. - Update roxygen2 documentation and metadata. --- R-package/DESCRIPTION | 2 +- R-package/R/xgboost.R | 39 +++++++++++++++---------- R-package/man/predict.xgboost.Rd | 4 ++- R-package/man/xgb.plot.deepness.Rd | 2 +- R-package/man/xgb.plot.shap.Rd | 2 +- R-package/tests/testthat/test_xgboost.R | 28 ++++++++++++++++++ 6 files changed, 58 insertions(+), 19 deletions(-) diff --git a/R-package/DESCRIPTION b/R-package/DESCRIPTION index add44a596401..13f3905b8a06 100644 --- a/R-package/DESCRIPTION +++ b/R-package/DESCRIPTION @@ -71,6 +71,6 @@ Imports: data.table (>= 1.9.6), jsonlite (>= 1.0) Roxygen: list(markdown = TRUE) -RoxygenNote: 7.3.3 Encoding: UTF-8 SystemRequirements: GNU make, C++17 +Config/roxygen2/version: 8.0.0 diff --git a/R-package/R/xgboost.R b/R-package/R/xgboost.R index 9bb1ebd1636c..79918eadbabe 100644 --- a/R-package/R/xgboost.R +++ b/R-package/R/xgboost.R @@ -1321,7 +1321,9 @@ xgboost <- function( #' #' Note that this check might add some sizable latency to the predictions, so it's #' recommended to disable it for performance-sensitive applications. -#' @param ... Not used. +#' @param ... Legacy boolean flags (e.g., \code{predcontrib}) are supported for +#' backward compatibility but are deprecated and map to the \code{type} argument. +#' Any other arguments passed to \code{...} will cause an error. #' @return Either a numeric vector (for 1D outputs), numeric matrix (for 2D outputs), numeric array #' (for 3D and higher), or `factor` (for class predictions). See documentation for parameter `type` #' for details about what the output type and shape will be. @@ -1362,26 +1364,33 @@ predict.xgboost <- function( predinteraction = "interaction" ) found_legacy <- intersect(names(dots), names(mapping)) - + if (length(found_legacy) > 0) { - legacy_arg <- found_legacy[1] - if (isTRUE(dots[[legacy_arg]])) { - type <- mapping[[legacy_arg]] - warning( - sprintf( - "Argument '%s' is deprecated. Please use type = '%s' instead.", - legacy_arg, type - ), - call. = FALSE - ) + for (legacy_arg in found_legacy) { + if (isTRUE(dots[[legacy_arg]])) { + type <- mapping[[legacy_arg]] + warning( + sprintf( + "Argument '%s' is deprecated. Please use type = '%s' instead.", + legacy_arg, type + ), + call. = FALSE + ) + break + } } - dots <- dots[setdiff(names(dots), names(mapping))] + dots[found_legacy] <- NULL } - + if (length(dots) > 0) { + dot_names <- names(dots) + if (is.null(dot_names)) { + dot_names <- rep("", length(dots)) + } + dot_names[dot_names == ""] <- "" stop( "predict.xgboost: arguments in '...' are not supported (", - paste(names(dots), collapse = ", "), ")." + paste(dot_names, collapse = ", "), ")." ) } } diff --git a/R-package/man/predict.xgboost.Rd b/R-package/man/predict.xgboost.Rd index 7b212bb25a24..44f05cbcebe6 100644 --- a/R-package/man/predict.xgboost.Rd +++ b/R-package/man/predict.xgboost.Rd @@ -109,7 +109,9 @@ Be aware that this only applies to column names and not to factor levels in cate Note that this check might add some sizable latency to the predictions, so it's recommended to disable it for performance-sensitive applications.} -\item{...}{Not used.} +\item{...}{Legacy boolean flags (e.g., \code{predcontrib}) are supported for +backward compatibility but are deprecated and map to the \code{type} argument. +Any other arguments passed to \code{...} will cause an error.} } \value{ Either a numeric vector (for 1D outputs), numeric matrix (for 2D outputs), numeric array diff --git a/R-package/man/xgb.plot.deepness.Rd b/R-package/man/xgb.plot.deepness.Rd index 1e1827e42384..d2d385ba0257 100644 --- a/R-package/man/xgb.plot.deepness.Rd +++ b/R-package/man/xgb.plot.deepness.Rd @@ -25,7 +25,7 @@ by \code{\link[=xgb.model.dt.tree]{xgb.model.dt.tree()}}.} \item{plot}{Should the plot be shown? Default is \code{TRUE}.} -\item{...}{Other parameters passed to \code{\link[graphics:barplot]{graphics::barplot()}} or \code{\link[graphics:plot.default]{graphics::plot()}}.} +\item{...}{Other parameters passed to \code{\link[graphics:barplot]{graphics::barplot()}} or \code{\link[graphics:plot]{graphics::plot()}}.} } \value{ The return value of the two functions is as follows: diff --git a/R-package/man/xgb.plot.shap.Rd b/R-package/man/xgb.plot.shap.Rd index 2d9563117528..efff6a39ccb6 100644 --- a/R-package/man/xgb.plot.shap.Rd +++ b/R-package/man/xgb.plot.shap.Rd @@ -94,7 +94,7 @@ The smoothing is only done for features with more than 5 distinct values.} \item{plot}{Should the plot be drawn? (Default is \code{TRUE}). If \code{FALSE}, only a list of matrices is returned.} -\item{...}{Other parameters passed to \code{\link[graphics:plot.default]{graphics::plot()}}.} +\item{...}{Other parameters passed to \code{\link[graphics:plot]{graphics::plot()}}.} } \value{ In addition to producing plots (when \code{plot = TRUE}), it silently returns a list of two matrices: diff --git a/R-package/tests/testthat/test_xgboost.R b/R-package/tests/testthat/test_xgboost.R index e70077092058..8706cb38db3b 100644 --- a/R-package/tests/testthat/test_xgboost.R +++ b/R-package/tests/testthat/test_xgboost.R @@ -1147,9 +1147,37 @@ test_that("predict.xgboost maps legacy boolean flags to type", { ) expect_equal(dim(pred), c(nrow(x), ncol(x) + 1L)) + # Test multiple flags (first is FALSE, second is TRUE) + expect_warning( + pred2 <- predict(model, x, outputmargin = FALSE, predinteraction = TRUE), + paste0( + "Argument 'predinteraction' is deprecated. ", + "Please use type = 'interaction' instead." + ), + fixed = TRUE + ) + expect_equal(dim(pred2), c(nrow(x), ncol(x) + 1L, ncol(x) + 1L)) + + # Test conflict between legacy flag and explicit type + expect_warning( + pred3 <- predict(model, x, type = "response", approxcontrib = TRUE), + paste0( + "Argument 'approxcontrib' is deprecated. ", + "Please use type = 'contrib' instead." + ), + fixed = TRUE + ) + expect_equal(dim(pred3), c(nrow(x), ncol(x) + 1L)) + + # Test unsupported arguments expect_error( predict(model, x, foobar = TRUE), "predict.xgboost: arguments in '...' are not supported (foobar).", fixed = TRUE ) + expect_error( + predict(model, x, "response", NULL, NULL, TRUE, "some_unnamed_arg"), + "predict.xgboost: arguments in '...' are not supported ().", + fixed = TRUE + ) })