Fail on lints (#932)

* Fail on lints * Update lint-changed-files.yaml * clean lints
IndrajeetPatil · Jul 23, 2024 · b16fe06 · b16fe06
1 parent df5721d
commit b16fe06
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 66 deletions.
diff --git a/.github/workflows/lint-changed-files.yaml b/.github/workflows/lint-changed-files.yaml
@@ -21,10 +21,12 @@ jobs:
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
           pak-version: devel
-          upgrade: 'TRUE'
+          upgrade: "TRUE"
           extra-packages: |
             any::gh
+            any::purrr
             r-lib/lintr
+          needs: check
 
       - name: Install package
         run: R CMD INSTALL .
@@ -37,12 +39,11 @@ 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 = linters_with_defaults(
+          lint_package(linters = all_linters(
             absolute_path_linter = NULL,
             assignment_linter = NULL,
             commented_code_linter = NULL,
             cyclocomp_linter(25L),
-            extraction_operator_linter = NULL,
             if_not_else_linter(exceptions = character(0L)),
             implicit_integer_linter = NULL,
             library_call_linter = NULL,
@@ -58,9 +59,8 @@ jobs:
             undesirable_function_linter = NULL,
             undesirable_operator_linter = NULL,
             unnecessary_concatenation_linter(allow_single_expression = FALSE),
-            unused_import_linter = NULL,
-            defaults = linters_with_tags(tags = NULL)
+            unused_import_linter = NULL
           ), exclusions = exclusions_list)
         shell: Rscript {0}
-        # env:
-        #   LINTR_ERROR_ON_LINT: true
+        env:
+          LINTR_ERROR_ON_LINT: true
diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml
@@ -27,7 +27,7 @@ jobs:
       - uses: r-lib/actions/setup-r-dependencies@v2
         with:
           pak-version: devel
-          upgrade: 'TRUE'
+          upgrade: "TRUE"
           extra-packages: |
             r-lib/lintr
             local::.
@@ -39,12 +39,11 @@ jobs:
         run: |
           options(crayon.enabled = TRUE)
           library(lintr)
-          lint_package(linters = linters_with_defaults(
+          lint_package(linters = all_linters(
             absolute_path_linter = NULL,
             assignment_linter = NULL,
             commented_code_linter = NULL,
             cyclocomp_linter(25L),
-            extraction_operator_linter = NULL,
             if_not_else_linter(exceptions = character(0L)),
             implicit_integer_linter = NULL,
             library_call_linter = NULL,
@@ -60,7 +59,8 @@ jobs:
             undesirable_function_linter = NULL,
             undesirable_operator_linter = NULL,
             unnecessary_concatenation_linter(allow_single_expression = FALSE),
-            unused_import_linter = NULL,
-            defaults = linters_with_tags(tags = NULL)
+            unused_import_linter = NULL
           ))
         shell: Rscript {0}
+        env:
+          LINTR_ERROR_ON_LINT: true
diff --git a/R/ggcoefstats.R b/R/ggcoefstats.R
@@ -173,33 +173,11 @@ ggcoefstats <- function(
     )
 
     # anova objects need further cleaning
+    # nolint next: line_length_linter.
     if (all(c("df", "df.error") %in% names(tidy_df))) tidy_df %<>% mutate(effectsize = paste0("partial ", effectsize.type, "-squared"))
   }
 
-  # tidy data frame cleanup -------------------------
-
-  if (is.null(tidy_df) || !"estimate" %in% names(tidy_df)) {
-    rlang::abort("The tidy data frame *must* contain 'estimate' column.")
-  }
-
-  # create a new term column if it's not present
-  if (!"term" %in% names(tidy_df)) tidy_df %<>% mutate(term = paste0("term_", row_number()))
-
-  # check for duplicate terms and columns -------------------------
-
-  # check if there are repeated terms (relevant for `maov`, `lqm`, etc.)
-  if (anyDuplicated(tidy_df$term)) {
-    tidy_df %<>%
-      tidyr::unite(
-        col    = "term",
-        matches("term|variable|parameter|method|curve|response|component|contrast|group"),
-        remove = TRUE,
-        sep    = "_"
-      )
-  }
-
-  # halt if there are still repeated terms
-  if (anyDuplicated(tidy_df$term)) rlang::abort("Elements in `term` column must be unique.")
+  tidy_df <- .preprocess_tidy_data(tidy_df, sort)
 
   # if tidy data frame doesn't contain p-value or statistic column, no label
   if (!(all(c("p.value", "statistic") %in% names(tidy_df)))) stats.labels <- FALSE
@@ -212,7 +190,6 @@ ggcoefstats <- function(
     conf.int <- FALSE
   }
 
-  # whether to show model intercept
   if (exclude.intercept) tidy_df %<>% filter(!grepl("(Intercept)", term, TRUE))
 
   # label -------------------------
@@ -226,19 +203,12 @@ ggcoefstats <- function(
     }
   }
 
-  # sorting -------------------------
-
-  tidy_df %<>% parameters::sort_parameters(sort = sort, column = "estimate")
-
-  # `term` needs to be a factor column; otherwise, ggplot2 will sort the `x`-axis
-  # labels alphabetically and terms won't appear in the expected order
-  tidy_df %<>% dplyr::mutate(term = factor(term, tidy_df$term))
-
   # summary caption -------------------------
 
   glance_df <- performance::model_performance(x, verbose = FALSE) %>% as_tibble()
 
   if (!is.null(glance_df) && all(c("AIC", "BIC") %in% names(glance_df))) {
+    # nolint next: line_length_linter.
     glance_df %<>% mutate(expression = list(parse(text = glue("list(AIC=='{format_value(AIC, 0L)}', BIC=='{format_value(BIC, 0L)}')"))))
     caption <- .extract_expression(glance_df)
   }
@@ -248,11 +218,9 @@ ggcoefstats <- function(
   if (meta.analytic.effect) {
     meta.type <- stats_type_switch(meta.type)
 
-    # frequentist
     subtitle_df <- meta_analysis(tidy_df, type = meta.type, digits = digits)
     subtitle <- .extract_expression(subtitle_df)
 
-    # Bayesian
     if (meta.type == "parametric" && bf.message) {
       caption_df <- suppressWarnings(meta_analysis(tidy_df, type = "bayes", digits = digits))
       caption <- .extract_expression(caption_df)
@@ -275,7 +243,6 @@ ggcoefstats <- function(
       )
   }
 
-  # adding the vertical line
   if (vline) plot_coef <- plot_coef + exec(geom_vline, xintercept = 0, !!!vline.args)
 
   # ggrepel labels -------------------------
@@ -296,7 +263,7 @@ ggcoefstats <- function(
       )
   }
 
-  # annotations -------------------------
+  # annotations ---------------------------------------------
 
   plot_coef +
     labs(
@@ -309,3 +276,34 @@ ggcoefstats <- function(
     ggtheme +
     theme(plot.caption = element_text(size = 10))
 }
+
+
+#' @noRd
+.preprocess_tidy_data <- function(data, sort) {
+  if (is.null(data) || !"estimate" %in% names(data)) {
+    rlang::abort("The tidy data frame *must* contain 'estimate' column.")
+  }
+
+  # create a new term column if it's not present
+  if (!"term" %in% names(data)) data %<>% mutate(term = paste0("term_", row_number()))
+
+  # check if there are repeated terms (relevant for `maov`, `lqm`, etc.)
+  if (anyDuplicated(data$term)) {
+    data %<>%
+      tidyr::unite(
+        col    = "term",
+        matches("term|variable|parameter|method|curve|response|component|contrast|group"),
+        remove = TRUE,
+        sep    = "_"
+      )
+  }
+
+  # halt if there are still repeated terms
+  if (anyDuplicated(data$term)) rlang::abort("Elements in `term` column must be unique.")
+
+  data %<>% parameters::sort_parameters(sort = sort, column = "estimate")
+
+  # `term` needs to be a factor column; otherwise, ggplot2 will sort the `x`-axis
+  # labels alphabetically and terms won't appear in the expected order
+  data %>% dplyr::mutate(term = factor(term, data$term))
+}
diff --git a/R/ggpiestats-ggbarstats-helpers.R b/R/ggpiestats-ggbarstats-helpers.R
@@ -37,18 +37,15 @@ descriptive_data <- function(
 #' @autoglobal
 #' @noRd
 onesample_data <- function(data, x, y, digits = 2L, ratio = NULL, ...) {
-  full_join(
-    # descriptive summary
-    x = .cat_counter(data, {{ y }}) %>%
-      mutate(N = paste0("(n = ", .prettyNum(counts), ")")),
-    # proportion test results
-    y = group_by(data, {{ y }}) %>%
-      group_modify(.f = ~ .chisq_test_safe(., {{ x }}, ratio)) %>%
-      ungroup(),
-    by = as_name(ensym(y))
-  ) %>%
+  grouped_chi_squared_summary <- group_by(data, {{ y }}) %>%
+    group_modify(.f = ~ .chisq_test_safe(., {{ x }}, ratio)) %>%
+    ungroup()
+  descriptive_summary <- .cat_counter(data, {{ y }}) %>% mutate(N = paste0("(n = ", .prettyNum(counts), ")"))
+
+  full_join(descriptive_summary, grouped_chi_squared_summary, by = as_name(ensym(y))) %>%
     rowwise() %>%
     mutate(
+      # nolint next: line_length_linter.
       .label = glue("list(~chi['gof']^2~({df})=={format_value(statistic, digits)}, ~italic(p)=='{format_value(p.value, digits)}', ~italic(n)=='{.prettyNum(counts)}')"),
       .p.label = glue("list(~italic(p)=='{format_value(p.value, digits)}')")
     ) %>%

diff --git a/README.md b/README.md
@@ -697,18 +697,12 @@ extract_subtitle(p)
 #> list(italic("F")["Welch"](2, 18.03) == "31.62", italic(p) == 
 #>     "1.27e-06", widehat(omega["p"]^2) == "0.74", CI["95%"] ~ 
 #>     "[" * "0.53", "1.00" * "]", italic("n")["obs"] == "32")
-```
-
-``` r
 
 # extracting expression present in the caption
 extract_caption(p)
 #> list(log[e] * (BF["01"]) == "-14.92", widehat(italic(R^"2"))["Bayesian"]^"posterior" == 
 #>     "0.71", CI["95%"]^HDI ~ "[" * "0.57", "0.79" * "]", italic("r")["Cauchy"]^"JZS" == 
 #>     "0.71")
-```
-
-``` r
 
 # a list of tibbles containing statistical analysis summaries
 extract_stats(p)

diff --git a/data-raw/movies-wide-long.R b/data-raw/movies-wide-long.R
@@ -4,7 +4,7 @@ library(magrittr)
 movies_wide <-
   ggplot2movies::movies %>%
   dplyr::select(c(title:votes, mpaa:Short)) %>%
-  dplyr::filter(mpaa != "", mpaa != "NC-17", Short != 1L, Documentary != 1L) %>%
+  dplyr::filter(nzchar(mpaa), mpaa != "NC-17", Short != 1L, Documentary != 1L) %>%
   dplyr::select(-c(Short, Documentary)) %>%
   tidyr::drop_na() %>%
   dplyr::mutate(