Skip to content

Commit b4e7e50

Browse files
dongjoon-hyunHyukjinKwon
authored andcommitted
[SPARK-29936][R][2.4] Fix SparkR lint errors and add lint-r GitHub Action
### What changes were proposed in this pull request? This PR fixes SparkR lint errors and adds `lint-r` GitHub Action to protect the branch. ### Why are the changes needed? It turns out that we currently don't run it. It's recovered yesterday. However, after that, our Jenkins linter jobs (`master`/`branch-2.4`) has been broken on `lint-r` tasks. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Pass the GitHub Action on this PR in addition to Jenkins R. Closes #26568 from dongjoon-hyun/SPARK-29936-2. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: HyukjinKwon <[email protected]>
1 parent ee6693e commit b4e7e50

File tree

10 files changed

+40
-17
lines changed

10 files changed

+40
-17
lines changed

.github/workflows/branch-2.4.yml

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ jobs:
5050
5151
lint:
5252
runs-on: ubuntu-latest
53-
name: Linters
53+
name: Linters (Java/Scala/Python), licenses, dependencies
5454
steps:
5555
- uses: actions/checkout@master
5656
- uses: actions/setup-java@v1
@@ -72,3 +72,26 @@ jobs:
7272
run: ./dev/check-license
7373
- name: Dependencies
7474
run: ./dev/test-dependencies.sh
75+
76+
lintr:
77+
runs-on: ubuntu-latest
78+
name: Linter (R)
79+
steps:
80+
- uses: actions/checkout@master
81+
- uses: actions/setup-java@v1
82+
with:
83+
java-version: '1.8'
84+
- name: install R
85+
run: |
86+
echo 'deb https://cloud.r-project.org/bin/linux/ubuntu bionic-cran35/' | sudo tee -a /etc/apt/sources.list
87+
sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys E298A3A825C0D65DFD57CBB651716619E084DAB9
88+
sudo apt-get update
89+
sudo apt-get install -y r-base r-base-dev libcurl4-openssl-dev
90+
- name: install R packages
91+
run: |
92+
sudo Rscript -e "install.packages(c('curl', 'xml2', 'httr', 'devtools', 'testthat', 'knitr', 'rmarkdown', 'roxygen2', 'e1071', 'survival'), repos='https://cloud.r-project.org/')"
93+
sudo Rscript -e "devtools::install_github('jimhester/[email protected]')"
94+
- name: package and install SparkR
95+
run: ./R/install-dev.sh
96+
- name: lint-r
97+
run: ./dev/lint-r

R/pkg/.lintr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
linters: with_defaults(line_length_linter(100), multiple_dots_linter = NULL, object_name_linter = NULL, camel_case_linter = NULL, open_curly_linter(allow_single_line = TRUE), closed_curly_linter(allow_single_line = TRUE))
1+
linters: with_defaults(line_length_linter(100), multiple_dots_linter = NULL, object_name_linter = NULL, camel_case_linter = NULL, open_curly_linter(allow_single_line = TRUE), closed_curly_linter(allow_single_line = TRUE), object_usage_linter = NULL, cyclocomp_linter = NULL)
22
exclusions: list("inst/profile/general.R" = 1, "inst/profile/shell.R")

R/pkg/R/DataFrame.R

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2198,7 +2198,7 @@ setMethod("mutate",
21982198

21992199
# The last column of the same name in the specific columns takes effect
22002200
deDupCols <- list()
2201-
for (i in 1:length(cols)) {
2201+
for (i in seq_len(length(cols))) {
22022202
deDupCols[[ns[[i]]]] <- alias(cols[[i]], ns[[i]])
22032203
}
22042204

@@ -2362,7 +2362,7 @@ setMethod("arrange",
23622362
# builds a list of columns of type Column
23632363
# example: [[1]] Column Species ASC
23642364
# [[2]] Column Petal_Length DESC
2365-
jcols <- lapply(seq_len(length(decreasing)), function(i){
2365+
jcols <- lapply(seq_len(length(decreasing)), function(i) {
23662366
if (decreasing[[i]]) {
23672367
desc(getColumn(x, by[[i]]))
23682368
} else {
@@ -2694,7 +2694,7 @@ genAliasesForIntersectedCols <- function(x, intersectedColNames, suffix) {
26942694
col <- getColumn(x, colName)
26952695
if (colName %in% intersectedColNames) {
26962696
newJoin <- paste(colName, suffix, sep = "")
2697-
if (newJoin %in% allColNames){
2697+
if (newJoin %in% allColNames) {
26982698
stop("The following column name: ", newJoin, " occurs more than once in the 'DataFrame'.",
26992699
"Please use different suffixes for the intersected columns.")
27002700
}
@@ -3393,7 +3393,7 @@ setMethod("str",
33933393
cat(paste0("'", class(object), "': ", length(names), " variables:\n"))
33943394

33953395
if (nrow(localDF) > 0) {
3396-
for (i in 1 : ncol(localDF)) {
3396+
for (i in seq_len(ncol(localDF))) {
33973397
# Get the first elements for each column
33983398

33993399
firstElements <- if (types[i] == "character") {

R/pkg/R/SQLContext.R

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ createDataFrame.default <- function(data, schema = NULL, samplingRatio = 1.0,
254254
as.list(schema)
255255
}
256256
if (is.null(names)) {
257-
names <- lapply(1:length(row), function(x) {
257+
names <- lapply(seq_len(length(row)), function(x) {
258258
paste("_", as.character(x), sep = "")
259259
})
260260
}
@@ -270,7 +270,7 @@ createDataFrame.default <- function(data, schema = NULL, samplingRatio = 1.0,
270270
})
271271

272272
types <- lapply(row, infer_type)
273-
fields <- lapply(1:length(row), function(i) {
273+
fields <- lapply(seq_len(length(row)), function(i) {
274274
structField(names[[i]], types[[i]], TRUE)
275275
})
276276
schema <- do.call(structType, fields)

R/pkg/R/context.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ spark.getSparkFiles <- function(fileName) {
412412
#' @examples
413413
#'\dontrun{
414414
#' sparkR.session()
415-
#' doubled <- spark.lapply(1:10, function(x){2 * x})
415+
#' doubled <- spark.lapply(1:10, function(x) {2 * x})
416416
#'}
417417
#' @note spark.lapply since 2.0.0
418418
spark.lapply <- function(list, func) {

R/pkg/R/group.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ methods <- c("avg", "max", "mean", "min", "sum")
162162
#' @note pivot since 2.0.0
163163
setMethod("pivot",
164164
signature(x = "GroupedData", colname = "character"),
165-
function(x, colname, values = list()){
165+
function(x, colname, values = list()) {
166166
stopifnot(length(colname) == 1)
167167
if (length(values) == 0) {
168168
result <- callJMethod(x@sgd, "pivot", colname)

R/pkg/R/utils.R

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ hashCode <- function(key) {
131131
} else {
132132
asciiVals <- sapply(charToRaw(key), function(x) { strtoi(x, 16L) })
133133
hashC <- 0
134-
for (k in 1:length(asciiVals)) {
134+
for (k in seq_len(length(asciiVals))) {
135135
hashC <- mult31AndAdd(hashC, asciiVals[k])
136136
}
137137
as.integer(hashC)
@@ -724,7 +724,7 @@ assignNewEnv <- function(data) {
724724
stopifnot(length(cols) > 0)
725725

726726
env <- new.env()
727-
for (i in 1:length(cols)) {
727+
for (i in seq_len(length(cols))) {
728728
assign(x = cols[i], value = data[, cols[i], drop = F], envir = env)
729729
}
730730
env
@@ -750,7 +750,7 @@ launchScript <- function(script, combinedArgs, wait = FALSE, stdout = "", stderr
750750
if (.Platform$OS.type == "windows") {
751751
scriptWithArgs <- paste(script, combinedArgs, sep = " ")
752752
# on Windows, intern = F seems to mean output to the console. (documentation on this is missing)
753-
shell(scriptWithArgs, translate = TRUE, wait = wait, intern = wait) # nolint
753+
shell(scriptWithArgs, translate = TRUE, wait = wait, intern = wait)
754754
} else {
755755
# http://stat.ethz.ch/R-manual/R-devel/library/base/html/system2.html
756756
# stdout = F means discard output

R/pkg/inst/worker/worker.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ if (isEmpty != 0) {
181181
colNames, computeFunc, data)
182182
} else {
183183
# gapply mode
184-
for (i in 1:length(data)) {
184+
for (i in seq_len(length(data))) {
185185
# Timing reading input data for execution
186186
inputElap <- elapsedSecs()
187187
output <- compute(mode, partition, serializer, deserializer, keys[[i]],

R/pkg/tests/fulltests/test_sparkSQL.R

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ test_that("structField type strings", {
181181
typeList <- c(primitiveTypes, complexTypes)
182182
typeStrings <- names(typeList)
183183

184-
for (i in seq_along(typeStrings)){
184+
for (i in seq_along(typeStrings)) {
185185
typeString <- typeStrings[i]
186186
expected <- typeList[[i]]
187187
testField <- structField("_col", typeString)
@@ -212,7 +212,7 @@ test_that("structField type strings", {
212212
errorList <- c(primitiveErrors, complexErrors)
213213
typeStrings <- names(errorList)
214214

215-
for (i in seq_along(typeStrings)){
215+
for (i in seq_along(typeStrings)) {
216216
typeString <- typeStrings[i]
217217
expected <- paste0("Unsupported type for SparkDataframe: ", errorList[[i]])
218218
expect_error(structField("_col", typeString), expected)

dev/lint-r.R

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ if (! library(SparkR, lib.loc = LOCAL_LIB_LOC, logical.return = TRUE)) {
2727
# Installs lintr from Github in a local directory.
2828
# NOTE: The CRAN's version is too old to adapt to our rules.
2929
if ("lintr" %in% row.names(installed.packages()) == FALSE) {
30-
devtools::install_github("jimhester/lintr@5431140")
30+
devtools::install_github("jimhester/lintr@v2.0.0")
3131
}
3232

3333
library(lintr)

0 commit comments

Comments
 (0)