From b16fe062beee0bad16d2864a77221acf37992d28 Mon Sep 17 00:00:00 2001 From: Indrajeet Patil Date: Tue, 23 Jul 2024 20:00:23 +0200 Subject: [PATCH] Fail on lints (#932) * Fail on lints * Update lint-changed-files.yaml * clean lints --- .github/workflows/lint-changed-files.yaml | 14 ++--- .github/workflows/lint.yaml | 10 ++-- R/ggcoefstats.R | 72 +++++++++++------------ R/ggpiestats-ggbarstats-helpers.R | 17 +++--- README.md | 6 -- data-raw/movies-wide-long.R | 2 +- 6 files changed, 55 insertions(+), 66 deletions(-) diff --git a/.github/workflows/lint-changed-files.yaml b/.github/workflows/lint-changed-files.yaml index f5503c236..913e3c2ee 100644 --- 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 index 6ee48e52c..3371f8017 100644 --- 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 index d15c9845c..7af6eda6d 100644 --- 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 index b7f5be22f..acc27ee08 100644 --- 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 index a04a1275f..93c824458 100644 --- 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 index 224972020..edb9aa263 100644 --- 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(