From c7e82b59143cc4726c87686f8b5705485665f682 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Sat, 24 Dec 2022 15:02:56 -0600 Subject: [PATCH] [R] enforce lintr checks (fixes #8012) (#8613) --- .github/workflows/r_tests.yml | 3 +-- R-package/R/xgb.ggplot.R | 2 +- R-package/R/xgb.plot.multi.trees.R | 4 ++-- R-package/R/xgb.plot.shap.R | 2 +- R-package/R/xgb.train.R | 2 +- R-package/demo/interaction_constraints.R | 2 +- R-package/man/xgb.plot.importance.Rd | 2 +- demo/kaggle-higgs/speedtest.R | 2 +- demo/kaggle-otto/otto_train_pred.R | 4 ++-- demo/kaggle-otto/understandingXGBoostModel.Rmd | 2 +- tests/ci_build/lint_r.R | 3 +-- 11 files changed, 13 insertions(+), 15 deletions(-) diff --git a/.github/workflows/r_tests.yml b/.github/workflows/r_tests.yml index ed3f63000..e93dfe7b2 100644 --- a/.github/workflows/r_tests.yml +++ b/.github/workflows/r_tests.yml @@ -44,8 +44,7 @@ jobs: - name: Run lintr run: | MAKEFLAGS="-j$(nproc)" R CMD INSTALL R-package/ - # Disable lintr errors for now: https://github.com/dmlc/xgboost/issues/8012 - Rscript tests/ci_build/lint_r.R $(pwd) || true + Rscript tests/ci_build/lint_r.R $(pwd) test-R-on-Windows: runs-on: ${{ matrix.config.os }} diff --git a/R-package/R/xgb.ggplot.R b/R-package/R/xgb.ggplot.R index 9f92c759b..f96a8a37f 100644 --- a/R-package/R/xgb.ggplot.R +++ b/R-package/R/xgb.ggplot.R @@ -4,7 +4,7 @@ #' @rdname xgb.plot.importance #' @export xgb.ggplot.importance <- function(importance_matrix = NULL, top_n = NULL, measure = NULL, - rel_to_first = FALSE, n_clusters = c(1:10), ...) { + rel_to_first = FALSE, n_clusters = seq_len(10), ...) { importance_matrix <- xgb.plot.importance(importance_matrix, top_n = top_n, measure = measure, rel_to_first = rel_to_first, plot = FALSE, ...) diff --git a/R-package/R/xgb.plot.multi.trees.R b/R-package/R/xgb.plot.multi.trees.R index 1b3a7d781..63c66008d 100644 --- a/R-package/R/xgb.plot.multi.trees.R +++ b/R-package/R/xgb.plot.multi.trees.R @@ -97,9 +97,9 @@ xgb.plot.multi.trees <- function(model, feature_names = NULL, features_keep = 5, , by = .(abs.node.position, Feature) ][, .(Text = paste0( paste0( - Feature[1:min(length(Feature), features_keep)], + Feature[seq_len(min(length(Feature), features_keep))], " (", - format(Quality[1:min(length(Quality), features_keep)], digits = 5), + format(Quality[seq_len(min(length(Quality), features_keep))], digits = 5), ")" ), collapse = "\n" diff --git a/R-package/R/xgb.plot.shap.R b/R-package/R/xgb.plot.shap.R index 8f8e921a8..7899dea7a 100644 --- a/R-package/R/xgb.plot.shap.R +++ b/R-package/R/xgb.plot.shap.R @@ -273,7 +273,7 @@ xgb.shap.data <- function(data, shap_contrib = NULL, features = NULL, top_n = 1, } top_n <- top_n[1] if (top_n < 1 | top_n > 100) stop("top_n: must be an integer within [1, 100]") - features <- imp$Feature[1:min(top_n, NROW(imp))] + features <- imp$Feature[seq_len(min(top_n, NROW(imp)))] } if (is.character(features)) { features <- match(features, colnames(data)) diff --git a/R-package/R/xgb.train.R b/R-package/R/xgb.train.R index a6e93b468..0841cbd69 100644 --- a/R-package/R/xgb.train.R +++ b/R-package/R/xgb.train.R @@ -389,7 +389,7 @@ xgb.train <- function(params = list(), data, nrounds, watchlist = list(), xgb.iter.update(bst$handle, dtrain, iteration - 1, obj) if (length(watchlist) > 0) - bst_evaluation <- xgb.iter.eval(bst$handle, watchlist, iteration - 1, feval) + bst_evaluation <- xgb.iter.eval(bst$handle, watchlist, iteration - 1, feval) # nolint: object_usage_linter xgb.attr(bst$handle, 'niter') <- iteration - 1 diff --git a/R-package/demo/interaction_constraints.R b/R-package/demo/interaction_constraints.R index c0db93fae..6da541b9b 100644 --- a/R-package/demo/interaction_constraints.R +++ b/R-package/demo/interaction_constraints.R @@ -33,7 +33,7 @@ treeInteractions <- function(input_tree, input_max_depth) { } # Extract nodes with interactions - interaction_trees <- trees[!is.na(Split) & !is.na(parent_1), + interaction_trees <- trees[!is.na(Split) & !is.na(parent_1), # nolint: object_usage_linter c('Feature', paste0('parent_feat_', 1:(input_max_depth - 1))), with = FALSE] interaction_trees_split <- split(interaction_trees, seq_len(nrow(interaction_trees))) diff --git a/R-package/man/xgb.plot.importance.Rd b/R-package/man/xgb.plot.importance.Rd index 691a8fdfc..1ee58b7ad 100644 --- a/R-package/man/xgb.plot.importance.Rd +++ b/R-package/man/xgb.plot.importance.Rd @@ -10,7 +10,7 @@ xgb.ggplot.importance( top_n = NULL, measure = NULL, rel_to_first = FALSE, - n_clusters = c(1:10), + n_clusters = seq_len(10), ... ) diff --git a/demo/kaggle-higgs/speedtest.R b/demo/kaggle-higgs/speedtest.R index 2794fdd75..c0e96a010 100644 --- a/demo/kaggle-higgs/speedtest.R +++ b/demo/kaggle-higgs/speedtest.R @@ -26,7 +26,7 @@ print(paste("weight statistics: wpos=", sumwpos, "wneg=", sumwneg, "ratio=", sum xgboost.time <- list() threads <- c(1, 2, 4, 8, 16) -for (i in 1:length(threads)){ +for (i in seq_along(threads)){ thread <- threads[i] xgboost.time[[i]] <- system.time({ xgmat <- xgb.DMatrix(data, label = label, weight = weight, missing = -999.0) diff --git a/demo/kaggle-otto/otto_train_pred.R b/demo/kaggle-otto/otto_train_pred.R index 7f54aebcd..9a5244754 100644 --- a/demo/kaggle-otto/otto_train_pred.R +++ b/demo/kaggle-otto/otto_train_pred.R @@ -13,7 +13,7 @@ y <- as.integer(y) - 1 # xgboost take features in [0,numOfClass) x <- rbind(train[, -ncol(train)], test) x <- as.matrix(x) x <- matrix(as.numeric(x), nrow(x), ncol(x)) -trind <- 1:length(y) +trind <- seq_along(y) teind <- (nrow(train) + 1):nrow(x) # Set necessary parameter @@ -43,6 +43,6 @@ pred <- t(pred) # Output submission pred <- format(pred, digits = 2, scientific = FALSE) # shrink the size of submission -pred <- data.frame(1:nrow(pred), pred) +pred <- data.frame(seq_len(nrow(pred)), pred) names(pred) <- c('id', paste0('Class_', 1:9)) write.csv(pred, file = 'submission.csv', quote = FALSE, row.names = FALSE) diff --git a/demo/kaggle-otto/understandingXGBoostModel.Rmd b/demo/kaggle-otto/understandingXGBoostModel.Rmd index 1939fa10c..d8f1ba663 100644 --- a/demo/kaggle-otto/understandingXGBoostModel.Rmd +++ b/demo/kaggle-otto/understandingXGBoostModel.Rmd @@ -30,7 +30,7 @@ require(xgboost) require(methods) require(data.table) require(magrittr) -train <- fread('data/train.csv', header = T, stringsAsFactors = FALSE) +train <- fread('data/train.csv', header = TRUE, stringsAsFactors = FALSE) test <- fread('data/test.csv', header = TRUE, stringsAsFactors = FALSE) ``` > `magrittr` and `data.table` are here to make the code cleaner and much more rapid. diff --git a/tests/ci_build/lint_r.R b/tests/ci_build/lint_r.R index d67fd16c5..43ede3cf3 100644 --- a/tests/ci_build/lint_r.R +++ b/tests/ci_build/lint_r.R @@ -67,6 +67,5 @@ noquote(paste0("Total linting issues found: ", issues_found)) if (issues_found > 0L) { print(results) + quit(save = "no", status = 1L) } - -quit(save = "no", status = 1L)