Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[R-package][ci] Added more R linters (fixes #2477) #2533

Merged
merged 22 commits into from
Nov 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions .ci/lint_r_code.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ library(lintr)
args <- commandArgs(
trailingOnly = TRUE
)
SOURCE_DIR <- args[[1]]
SOURCE_DIR <- args[[1L]]

FILES_TO_LINT <- list.files(
path = SOURCE_DIR
Expand All @@ -17,22 +17,34 @@ FILES_TO_LINT <- list.files(
)

LINTERS_TO_USE <- list(
"closed_curly" = lintr::closed_curly_linter
"assignment" = lintr::assignment_linter
, "closed_curly" = lintr::closed_curly_linter
, "equals_na" = lintr::equals_na_linter
, "function_left" = lintr::function_left_parentheses_linter
, "commas" = lintr::commas_linter
, "concatenation" = lintr::unneeded_concatenation_linter
, "implicit_integers" = lintr::implicit_integer_linter
, "infix_spaces" = lintr::infix_spaces_linter
, "long_lines" = lintr::line_length_linter(length = 120)
, "long_lines" = lintr::line_length_linter(length = 120L)
, "tabs" = lintr::no_tab_linter
, "open_curly" = lintr::open_curly_linter
, "paren_brace_linter" = lintr::paren_brace_linter
, "semicolon" = lintr::semicolon_terminator_linter
, "seq" = lintr::seq_linter
, "single_quotes" = lintr::single_quotes_linter
, "spaces_inside" = lintr::spaces_inside_linter
, "spaces_left_parens" = lintr::spaces_left_parentheses_linter
, "todo_comments" = lintr::todo_comment_linter
, "trailing_blank" = lintr::trailing_blank_lines_linter
, "trailing_white" = lintr::trailing_whitespace_linter
, "true_false" = lintr::T_and_F_symbol_linter
)

cat(sprintf("Found %i R files to lint\n", length(FILES_TO_LINT)))

results <- c()
results <- NULL

for (r_file in FILES_TO_LINT){
for (r_file in FILES_TO_LINT) {

this_result <- lintr::lint(
filename = r_file
Expand All @@ -52,7 +64,7 @@ for (r_file in FILES_TO_LINT){

issues_found <- length(results)

if (issues_found > 0){
if (issues_found > 0L) {
cat("\n")
print(results)
}
Expand Down
7 changes: 5 additions & 2 deletions .ci/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,14 @@ if [[ $TRAVIS == "true" ]] && [[ $TASK == "check-docs" ]]; then
exit 0
fi

if [[ $TASK == "lint" ]]; then
if [[ $TRAVIS == "true" ]] && [[ $TASK == "lint" ]]; then
conda install -q -y -n $CONDA_ENV \
pycodestyle \
pydocstyle \
r-lintr
r-stringi # stringi needs to be installed separate from r-lintr to avoid issues like 'unable to load shared object stringi.so'
conda install -q -y -n $CONDA_ENV \
-c conda-forge \
r-lintr>=2.0
pip install --user cpplint
echo "Linting Python code"
pycodestyle --ignore=E501,W503 --exclude=./compute,./.nuget . || exit -1
Expand Down
2 changes: 1 addition & 1 deletion R-package/R/aliases.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# lazy evaluation (so it doesn't matter what order R sources files during installation).
# [return] A named list, where each key is a main LightGBM parameter and each value is a character
# vector of corresponding aliases.
.PARAMETER_ALIASES <- function(){
.PARAMETER_ALIASES <- function() {
return(list(
"boosting" = c(
"boosting"
Expand Down
30 changes: 15 additions & 15 deletions R-package/R/callback.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ CB_ENV <- R6::R6Class(
end_iteration = NULL,
eval_list = list(),
eval_err_list = list(),
best_iter = -1,
best_iter = -1L,
best_score = NA,
met_early_stop = FALSE
)
Expand All @@ -30,7 +30,7 @@ cb.reset.parameters <- function(new_params) {
init <- function(env) {

# Store boosting rounds
nrounds <<- env$end_iteration - env$begin_iteration + 1
nrounds <<- env$end_iteration - env$begin_iteration + 1L

# Check for model environment
if (is.null(env$model)) { stop("Env should have a ", sQuote("model")) }
Expand Down Expand Up @@ -60,7 +60,7 @@ cb.reset.parameters <- function(new_params) {
if (is.function(p)) {

# Check if requires at least two arguments
if (length(formals(p)) != 2) {
if (length(formals(p)) != 2L) {
stop("Parameter ", sQuote(n), " is a function but not of two arguments")
}

Expand Down Expand Up @@ -117,7 +117,7 @@ cb.reset.parameters <- function(new_params) {
format.eval.string <- function(eval_res, eval_err = NULL) {

# Check for empty evaluation string
if (is.null(eval_res) || length(eval_res) == 0) {
if (is.null(eval_res) || length(eval_res) == 0L) {
stop("no evaluation results")
}

Expand All @@ -133,15 +133,15 @@ format.eval.string <- function(eval_res, eval_err = NULL) {
merge.eval.string <- function(env) {

# Check length of evaluation list
if (length(env$eval_list) <= 0) {
if (length(env$eval_list) <= 0L) {
return("")
}

# Get evaluation
msg <- list(sprintf("[%d]:", env$iteration))

# Set if evaluation error
is_eval_err <- length(env$eval_err_list) > 0
is_eval_err <- length(env$eval_err_list) > 0L

# Loop through evaluation list
for (j in seq_along(env$eval_list)) {
Expand All @@ -162,25 +162,25 @@ merge.eval.string <- function(env) {

}

cb.print.evaluation <- function(period = 1) {
cb.print.evaluation <- function(period = 1L) {

# Create callback
callback <- function(env) {

# Check if period is at least 1 or more
if (period > 0) {
if (period > 0L) {

# Store iteration
i <- env$iteration

# Check if iteration matches moduo
if ( (i - 1) %% period == 0 || is.element(i, c(env$begin_iteration, env$end_iteration))) {
if ((i - 1L) %% period == 0L || is.element(i, c(env$begin_iteration, env$end_iteration))) {

# Merge evaluation string
msg <- merge.eval.string(env)

# Check if message is existing
if (nchar(msg) > 0) {
if (nchar(msg) > 0L) {
cat(merge.eval.string(env), "\n")
}

Expand All @@ -205,15 +205,15 @@ cb.record.evaluation <- function() {
callback <- function(env) {

# Return empty if empty evaluation list
if (length(env$eval_list) <= 0) {
if (length(env$eval_list) <= 0L) {
return()
}

# Set if evaluation error
is_eval_err <- length(env$eval_err_list) > 0
is_eval_err <- length(env$eval_err_list) > 0L

# Check length of recorded evaluation
if (length(env$model$record_evals) == 0) {
if (length(env$model$record_evals) == 0L) {

# Loop through each evaluation list element
for (j in seq_along(env$eval_list)) {
Expand Down Expand Up @@ -290,7 +290,7 @@ cb.early.stop <- function(stopping_rounds, verbose = TRUE) {
eval_len <<- length(env$eval_list)

# Early stopping cannot work without metrics
if (eval_len == 0) {
if (eval_len == 0L) {
stop("For early stopping, valids must have at least one element")
}

Expand All @@ -301,7 +301,7 @@ cb.early.stop <- function(stopping_rounds, verbose = TRUE) {

# Maximization or minimization task
factor_to_bigger_better <<- rep.int(1.0, eval_len)
best_iter <<- rep.int(-1, eval_len)
best_iter <<- rep.int(-1L, eval_len)
best_score <<- rep.int(-Inf, eval_len)
best_msg <<- list()

Expand Down
Loading