Skip to content

Commit

Permalink
[ci] re-enable all {lintr} linters (#233)
Browse files Browse the repository at this point in the history
  • Loading branch information
jameslamb authored Jan 28, 2025
1 parent ffa29b0 commit 6259c3d
Show file tree
Hide file tree
Showing 14 changed files with 277 additions and 292 deletions.
15 changes: 7 additions & 8 deletions .ci/lint_r_code.R
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,15 @@ interactive_text <- paste0(
)

LINTERS_TO_USE <- list(
# "absolute_path" = lintr::absolute_path_linter()
"assignment" = lintr::assignment_linter()
# , "closed_curly" = lintr::closed_curly_linter()
"absolute_path" = lintr::absolute_path_linter()
, "assignment" = lintr::assignment_linter()
, "braces" = lintr::brace_linter()
, "commas" = lintr::commas_linter()
, "equals_na" = lintr::equals_na_linter()
, "function_left" = lintr::function_left_parentheses_linter()
, "infix_spaces" = lintr::infix_spaces_linter()
, "no_tabs" = lintr::no_tab_linter()
# , "non_portable_path" = lintr::nonportable_path_linter()
# , "open_curly" = lintr::open_curly_linter()
, "semicolon" = lintr::semicolon_terminator_linter()
, "non_portable_path" = lintr::nonportable_path_linter()
, "semicolon" = lintr::semicolon_linter()
, "seq" = lintr::seq_linter()
, "spaces_inside" = lintr::spaces_inside_linter()
, "spaces_left_parens" = lintr::spaces_left_parentheses_linter()
Expand Down Expand Up @@ -72,7 +70,8 @@ LINTERS_TO_USE <- list(
, "??" = interactive_text
)
)
, "unneeded_concatenation" = lintr::unneeded_concatenation_linter()
, "unnecessary_concatenation" = lintr::unnecessary_concatenation_linter()
, "whitespace" = lintr::whitespace_linter()
)

cat(sprintf("Found %i R files to lint\n", length(FILES_TO_LINT)))
Expand Down
1 change: 1 addition & 0 deletions .ci/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ sudo apt-get install \
texlive-latex-recommended \
texlive-fonts-recommended \
texlive-fonts-extra \
tidy \
qpdf

Rscript -e "install.packages(c('assertthat', 'covr', 'data.table', 'futile.logger', 'httr', 'jsonlite', 'knitr', 'lintr', 'purrr', 'rmarkdown', 'stringr', 'testthat', 'uuid'), repos = 'https://cran.r-project.org', Ncpus = parallel::detectCores())"
Expand Down
9 changes: 9 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ on:
branches:
- main

# automatically cancel in-progress builds if another commit is pushed
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

env:
# parallelize compilation (extra important for Linux, where CRAN doesn't supply pre-compiled binaries)
MAKEFLAGS: "-j4"

jobs:
test:
name: test (ES ${{ matrix.es_version }})
Expand Down
40 changes: 24 additions & 16 deletions r-pkg/R/chomp_aggs.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ chomp_aggs <- function(aggs_json = NULL) {
)

# Gross special-case handler for one-level extended_stats aggregation
if (.IsExtendedStatsAgg(jsonList[["aggregations"]][[aggNames]])){
if (.IsExtendedStatsAgg(jsonList[["aggregations"]][[aggNames]])) {
log_info("es_search is assuming that this result is a one-level 'extended_stats' result.")
jsonList[["aggregations"]][[1]][["std_deviation_bounds.upper"]] <- jsonList[["aggregations"]][[1]][["std_deviation_bounds"]][["upper"]]
jsonList[["aggregations"]][[1]][["std_deviation_bounds.lower"]] <- jsonList[["aggregations"]][[1]][["std_deviation_bounds"]][["lower"]]
jsonList[["aggregations"]][[1]][["std_deviation_bounds"]] <- NULL
}

# Gross special-case handler for one-level percentiles aggregation
if (.IsPercentilesAgg(jsonList[["aggregations"]][[aggNames]])){
if (.IsPercentilesAgg(jsonList[["aggregations"]][[aggNames]])) {
log_info("es_search is assuming that this result is a one-level 'percentiles' result.")

# Replace names like `25.0` with something that will be easier for users to understand
Expand All @@ -66,7 +66,7 @@ chomp_aggs <- function(aggs_json = NULL) {
jsonList[["aggregations"]][[aggNames]] <- percValues
}

if (.IsSigTermsAgg(jsonList[["aggregations"]][[aggNames]])){
if (.IsSigTermsAgg(jsonList[["aggregations"]][[aggNames]])) {
log_info("es_search is assuming that this result is a one-level 'significant terms' result.")

# We can grab that nested data.frame and break out right now
Expand All @@ -76,7 +76,7 @@ chomp_aggs <- function(aggs_json = NULL) {
}

# check for an empty result
if (identical(jsonList[["aggregations"]][[aggNames]][["buckets"]], list())){
if (identical(jsonList[["aggregations"]][[aggNames]][["buckets"]], list())) {
log_info("this aggregation result was empty. Returning NULL")
return(invisible(NULL))
}
Expand All @@ -96,7 +96,7 @@ chomp_aggs <- function(aggs_json = NULL) {
} else {

# Other bucketed aggregations (not date_histogram) will have "key"
if ("key" %in% names(outDT)){
if ("key" %in% names(outDT)) {
data.table::setnames(outDT, "key", aggNames[length(aggNames)])
} else {
# If we get down here, we know it's not a bucketed aggregation
Expand All @@ -115,7 +115,7 @@ chomp_aggs <- function(aggs_json = NULL) {

# Remove unwanted columns
badCols <- grep("doc_count", names(outDT))
if (length(badCols) > 0){
if (length(badCols) > 0) {
outDT <- outDT[, !badCols, with = FALSE]
}

Expand All @@ -135,7 +135,7 @@ chomp_aggs <- function(aggs_json = NULL) {
# Re-set the column order to mirror the way the user specified their aggs query
# NOTE: If there's no "doc_count" in the names, we know that this was not a bucketed
# / nested query and reordering is unnecessary
if ("doc_count" %in% names(outDT)){
if ("doc_count" %in% names(outDT)) {
data.table::setcolorder(
outDT,
c(aggNames, base::setdiff(names(outDT), c(aggNames, "doc_count")), "doc_count")
Expand All @@ -160,7 +160,7 @@ chomp_aggs <- function(aggs_json = NULL) {
# "extended_stats" aggregation. data.table doesn't handle those
# in a way that's consistent with the way this package handles all other aggregations
# [param] aggsList R list-object representation of an "aggs" result from Elasticsearch
.IsExtendedStatsAgg <- function(aggsList){
.IsExtendedStatsAgg <- function(aggsList) {
statsNames <- c("count", "min", "max", "avg", "sum", "sum_of_squares"
, "variance", "std_deviation", "std_deviation_bounds")

Expand All @@ -172,16 +172,24 @@ chomp_aggs <- function(aggs_json = NULL) {
# "Percentiles" aggregation. data.table doesn't handle those
# in a way that's consistent with the way this package handles all other aggregations
# [param] aggsList R list-object representation of an "aggs" result from Elasticsearch
.IsPercentilesAgg <- function(aggsList){
.IsPercentilesAgg <- function(aggsList) {

# check 1 - has a single element called "values"
if (! identical("values", names(aggsList))){
if (! identical("values", names(aggsList))) {
return(FALSE)
}

# check 2 - all names of "values" are convertible to numbers
numNames <- as.numeric(names(aggsList[["values"]]))
if (all(vapply(numNames, function(val){!is.na(val)}, FUN.VALUE = TRUE))){
all_convertible <- all(
vapply(
X = as.numeric(names(aggsList[["values"]]))
, FUN = function(val) {
return(!is.na(val))
}
, FUN.VALUE = TRUE
)
)
if (all_convertible) {
return(TRUE)
} else {
return(FALSE)
Expand All @@ -194,20 +202,20 @@ chomp_aggs <- function(aggs_json = NULL) {
# "significant terms" aggregation. data.table doesn't handle those
# in a way that's consistent with the way this package handles all other aggregations
# [param] aggsList R list-object representation of an "aggs" result from Elasticsearch
.IsSigTermsAgg <- function(aggsList){
.IsSigTermsAgg <- function(aggsList) {

# check 1 - has exactly two keys - "doc_count", "buckets"
if (! identical(sort(names(aggsList)), c('buckets', 'doc_count'))){
if (! identical(sort(names(aggsList)), c('buckets', 'doc_count'))) {
return(FALSE)
}

# check 2 - "buckets" is a data.frame
if (!is.data.frame(aggsList[['buckets']])){
if (!is.data.frame(aggsList[['buckets']])) {
return(FALSE)
}

# check 3 - "buckets" has at least the columns "key", "doc_count", and "bg_count"
if (!all(c('key', 'doc_count', 'bg_count') %in% names(aggsList[['buckets']]))){
if (!all(c('key', 'doc_count', 'bg_count') %in% names(aggsList[['buckets']]))) {
return(FALSE)
}

Expand Down
Loading

0 comments on commit 6259c3d

Please sign in to comment.