From ff5457b7e8102f2a6860637e4c6a28c65dfa06f5 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Fri, 26 Jul 2024 20:58:54 +0200 Subject: [PATCH] Use lintr config file also in GHA (#954) * Use lintr config file also in GHA * fix lints --- .github/workflows/lint-changed-files.yaml | 32 ++++------------------- .github/workflows/lint.yaml | 26 ++---------------- .lintr | 4 +-- R/ggpiestats.R | 2 +- tests/testthat/test-ggbarstats.R | 1 - tests/testthat/test-gghistostats.R | 9 ------- tests/testthat/test-ggpiestats.R | 1 - tests/testthat/test-ggscatterstats.R | 14 ---------- 8 files changed, 9 insertions(+), 80 deletions(-) diff --git a/.github/workflows/lint-changed-files.yaml b/.github/workflows/lint-changed-files.yaml index 913e3c2ee..07492f62f 100644 --- a/.github/workflows/lint-changed-files.yaml +++ b/.github/workflows/lint-changed-files.yaml @@ -21,15 +21,13 @@ jobs: - uses: r-lib/actions/setup-r-dependencies@v2 with: pak-version: devel - upgrade: "TRUE" + upgrade: 'TRUE' + needs: check extra-packages: | any::gh any::purrr r-lib/lintr - needs: check - - - name: Install package - run: R CMD INSTALL . + local::. - name: Extract and lint files changed by this PR run: | @@ -39,28 +37,8 @@ jobs: changed_files <- purrr::map_chr(files, "filename") all_files <- list.files(recursive = TRUE) exclusions_list <- as.list(setdiff(all_files, changed_files)) - lint_package(linters = all_linters( - absolute_path_linter = NULL, - assignment_linter = NULL, - commented_code_linter = NULL, - cyclocomp_linter(25L), - if_not_else_linter(exceptions = character(0L)), - implicit_integer_linter = NULL, - library_call_linter = NULL, - line_length_linter(120L), - namespace_linter = NULL, - nonportable_path_linter = NULL, - object_name_linter = NULL, - object_length_linter(50L), - object_usage_linter = NULL, - string_boundary_linter = NULL, - strings_as_factors_linter = NULL, - todo_comment_linter = NULL, - undesirable_function_linter = NULL, - undesirable_operator_linter = NULL, - unnecessary_concatenation_linter(allow_single_expression = FALSE), - unused_import_linter = NULL - ), exclusions = exclusions_list) + # lintr config in `.lintr` file + lint_package(exclusions = exclusions_list) shell: Rscript {0} env: LINTR_ERROR_ON_LINT: true diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 3371f8017..a44ebbe48 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -33,34 +33,12 @@ jobs: local::. needs: lint - # TODO: Revisit to remove some of these allowances after more important lints - # have been removed. - name: Lint run: | options(crayon.enabled = TRUE) library(lintr) - lint_package(linters = all_linters( - absolute_path_linter = NULL, - assignment_linter = NULL, - commented_code_linter = NULL, - cyclocomp_linter(25L), - if_not_else_linter(exceptions = character(0L)), - implicit_integer_linter = NULL, - library_call_linter = NULL, - line_length_linter(120L), - namespace_linter = NULL, - nonportable_path_linter = NULL, - object_name_linter = NULL, - object_length_linter(50L), - object_usage_linter = NULL, - string_boundary_linter = NULL, - strings_as_factors_linter = NULL, - todo_comment_linter = NULL, - undesirable_function_linter = NULL, - undesirable_operator_linter = NULL, - unnecessary_concatenation_linter(allow_single_expression = FALSE), - unused_import_linter = NULL - )) + # lintr config in `.lintr` file + lint_package() shell: Rscript {0} env: LINTR_ERROR_ON_LINT: true diff --git a/.lintr b/.lintr index a7324e54e..7467079e6 100644 --- a/.lintr +++ b/.lintr @@ -12,11 +12,9 @@ linters: all_linters( object_name_linter = NULL, object_length_linter(50L), object_usage_linter = NULL, - todo_comment_linter = NULL, string_boundary_linter = NULL, - strings_as_factors_linter = NULL, + strings_as_factors_linter = NULL, # since R > 4.0 undesirable_function_linter = NULL, - undesirable_operator_linter = NULL, unnecessary_concatenation_linter(allow_single_expression = FALSE), unused_import_linter = NULL ) diff --git a/R/ggpiestats.R b/R/ggpiestats.R index 71e1b39aa..21883e52f 100644 --- a/R/ggpiestats.R +++ b/R/ggpiestats.R @@ -114,7 +114,7 @@ ggpiestats <- function( x_levels <- nlevels(pull(data, {{ x }})) y_levels <- ifelse(test == "one.way", 0L, nlevels(pull(data, {{ y }}))) - # TODO: one-way table in `BayesFactor` (richarddmorey/BayesFactor#159) + # one-way table not supported in `BayesFactor` ATM (richarddmorey/BayesFactor#159) if (test == "two.way" && y_levels == 1L) bf.message <- FALSE # faceting is possible only if both vars have more than one level diff --git a/tests/testthat/test-ggbarstats.R b/tests/testthat/test-ggbarstats.R index d3c6f69d6..6e9a9b914 100644 --- a/tests/testthat/test-ggbarstats.R +++ b/tests/testthat/test-ggbarstats.R @@ -162,7 +162,6 @@ test_that( # dropped level dataset mtcars_small <- dplyr::filter(mtcars, am == "0") - # TODO: should one-way table results be shown in the subtitle? set.seed(123) expect_doppelganger( title = "works with dropped levels", diff --git a/tests/testthat/test-gghistostats.R b/tests/testthat/test-gghistostats.R index 479b2c0dd..b805895e8 100644 --- a/tests/testthat/test-gghistostats.R +++ b/tests/testthat/test-gghistostats.R @@ -86,15 +86,6 @@ test_that( } ) -# utilities --------------- - -test_that( - ".binwidth works as expected", - { - expect_equal(ggstatsplot:::.binwidth(mtcars$wt), 0.6913737, tolerance = 0.001) - } -) - # grouped_gghistostats works --------------------------------------------- test_that( diff --git a/tests/testthat/test-ggpiestats.R b/tests/testthat/test-ggpiestats.R index dc3124070..75b0c4fab 100644 --- a/tests/testthat/test-ggpiestats.R +++ b/tests/testthat/test-ggpiestats.R @@ -148,7 +148,6 @@ test_that( # dropped level dataset mtcars_small <- dplyr::filter(mtcars, am == "0") - # TODO: should one-way table results be shown in the subtitle? set.seed(123) expect_doppelganger( title = "works with dropped levels", diff --git a/tests/testthat/test-ggscatterstats.R b/tests/testthat/test-ggscatterstats.R index f0ee9624d..7e52a2bf9 100644 --- a/tests/testthat/test-ggscatterstats.R +++ b/tests/testthat/test-ggscatterstats.R @@ -75,20 +75,6 @@ test_that("labeling variables and expressions work as expected", { results.subtitle = FALSE ) ) - - # TODO: generate snapshot on Windows machine - # set.seed(123) - # expect_doppelganger( - # title = "label variable but no expression", - # fig = ggscatterstats( - # data = df, - # x = sleep_total, - # y = sleep_cycle, - # label.expression = NULL, - # label.var = order, - # results.subtitle = FALSE - # ) - # ) }) # subtitle output ----------------------------------------------------------