Find package improvements with `goodpractice` package

[sbreitbart-NOAA]

The goodpractice package gives "advice about good practices when building R packages. Advice includes functions and syntax to avoid, package structure, code complexity, code formatting, etc."

Steps:

1. Install the goodpractice and stockplotr R packages

install.packages("pak")
pak::pak("goodpractice")
pak::pak("nmfs-ost/stockplotr")

2. Find where the stockplotr package is saved on your machine. This workflow should work:

# Find where your packages live
pkg_location <- .libPaths()
# Then find where stockplotr lives, specifically. It might be:
stockplotr_location <- fs::path(pkg_location, "stockplotr")

3. Run goodpractice

goodpractice::gp("stockplotr_location")

4. Address the package's advice 🙂

[sbreitbart-NOAA]

Now, the goodpractice package has a guide that instructs AI agents how to review the package. Another way to address this issue is to share this guide with an agent and ask it to review the package based on the guide, then you review the resultant changes. When you've reviewed and edited the agent's changes, you can open a pull request for stockplotr.

[e-perl-NOAA]

Some of this has been implemented but the following list of things to be addressed is what I produced, AI agents were less helpful than expected - the branch is docs-goodpractice. This is not a hard issue but it does take a long time, so I would recommend putting that somewhere either in a category in an attached project board or as a label.

Scanning roxygen comments in R/ files...

Possible missing @noRd for internal helper:
  File: R/afsc_risk_table.R 
  Function: # risk_table <- function(dat, end_year, tier, c1, c2, c3) { 

Possible missing @param docs:
  File: R/afsc_risk_table.R 
  Function: # risk_table <- function(dat, end_year, tier, c1, c2, c3) { 
  Missing: dat, end_year, tier, c1, c2, c3) { #   # for cells in dat with numeric values, format the numbers #   # with built-in comma function #   for (column in names(dat)) { #     if (is.numeric(dat[[column]])) { #       dat[[column]] <- sapply(dat[[column]], comma 

Possible missing @noRd for internal helper:
  File: R/convert_output.R 
  Function: find_error_value 

Possible missing @param docs:
  File: R/convert_output.R 
  Function: find_error_value 
  Missing: err) {                         pattern <- paste0("(^|[_.])", err, "($|[_.])")                         if (grepl(pattern, col_name)) err else NA_character_                       }, FUN.VALUE 

Possible missing @param docs:
  File: R/html_all_figs_tables.R 
  Function: html_all_figs_tables 
  Missing: "figures"))) {     cli::cli_alert_danger("'figures' folder not found.")     cli::cli_alert_warning("Figures will not be included in the html.")     cli::cli_alert_info("Did you enter the correct argument for figures_tables_dir?")     cli::cli_alert_info("figures_tables_dir entered as {figures_tables_dir}" 

Possible missing @param docs:
  File: R/plot_abundance_at_age.R 
  Function: plot_abundance_at_age 
  Missing: plot_abundance_at_age <- function(   dat, facet, unit_label, scale_amount, proportional 

Possible missing @param docs:
  File: R/plot_biomass.R 
  Function: plot_biomass 
  Missing: plot_biomass <- function(   dat, geom, group, facet 

Function uses @inheritParams:
  File: R/plot_biomass.R 
  Function: plot_biomass 

Possible missing @param docs:
  File: R/plot_biomass_at_age.R 
  Function: plot_biomass_at_age 
  Missing: plot_biomass_at_age <- function(   dat, facet, unit_label, proportional 

Function uses @inheritParams:
  File: R/plot_biomass_at_age.R 
  Function: plot_biomass_at_age 

Possible missing @param docs:
  File: R/plot_catch_comp.R 
  Function: plot_catch_comp 
  Missing: plot_catch_comp <- function(   dat, facet 

Function uses @inheritParams:
  File: R/plot_catch_comp.R 
  Function: plot_catch_comp 

Possible missing @param docs:
  File: R/plot_discard.R 
  Function: plot_discard 
  Missing: plot_discard <- function(   dat, group, facet, unit_label, era 

Function uses @inheritParams:
  File: R/plot_discard.R 
  Function: plot_discard 

Possible missing @param docs:
  File: R/plot_fishing_mortality.R 
  Function: plot_fishing_mortality 
  Missing: plot_fishing_mortality <- function(   dat, geom, group, facet, ref_line 

Function uses @inheritParams:
  File: R/plot_fishing_mortality.R 
  Function: plot_fishing_mortality 

Possible missing @param docs:
  File: R/plot_index.R 
  Function: plot_index 
  Missing: plot_index <- function(   dat, group, # facet always assigned to fleet since that is how indices are calc'd -- unless replaced with NULL   facet 

Function uses @inheritParams:
  File: R/plot_index.R 
  Function: plot_index 

Possible missing @param docs:
  File: R/plot_landings.R 
  Function: plot_landings 
  Missing: plot_landings <- function(   dat, unit_label, geom, group, facet 

Function uses @inheritParams:
  File: R/plot_landings.R 
  Function: plot_landings 

Possible missing @param docs:
  File: R/plot_natural_mortality.R 
  Function: plot_natural_mortality 
  Missing: plot_natural_mortality <- function(   dat, group, facet, era, interactive 

Function uses @inheritParams:
  File: R/plot_natural_mortality.R 
  Function: plot_natural_mortality 

Possible missing @param docs:
  File: R/plot_recruitment.R 
  Function: plot_recruitment 
  Missing: plot_recruitment <- function(   dat, scale_amount, era, group 

Function uses @inheritParams:
  File: R/plot_recruitment.R 
  Function: plot_recruitment 

Possible missing @param docs:
  File: R/plot_recruitment_deviations.R 
  Function: plot_recruitment_deviations 
  Missing: plot_recruitment_deviations <- function(   dat, module, era, interactive, make_rda 

Function uses @inheritParams:
  File: R/plot_recruitment_deviations.R 
  Function: plot_recruitment_deviations 

Possible missing @param docs:
  File: R/plot_spawning_biomass.R 
  Function: plot_spawning_biomass 
  Missing: plot_spawning_biomass <- function(   dat 

Possible missing @param docs:
  File: R/plot_stock_recruitment.R 
  Function: plot_stock_recruitment 
  Missing: plot_stock_recruitment <- function(   dat, era 

Function uses @inheritParams:
  File: R/plot_stock_recruitment.R 
  Function: plot_stock_recruitment 

Possible missing @param docs:
  File: R/process_data.R 
  Function: process_data 
  Missing: process_data <- function(   dat 

Possible missing @param docs:
  File: R/process_data.R 
  Function: process_table 
  Missing: process_table <- function(   dat, group, method 

Function uses @inheritParams:
  File: R/process_data.R 
  Function: process_table 

Possible missing @param docs:
  File: R/save_all_plots.R 
  Function: save_all_plots 
  Missing: save_all_plots <- function(   # imported from plot_recruitment   dat, # changed from unit_label to recruitment_unit_label for specificity   recruitment_scale_amount, relative 

Function uses @inheritParams:
  File: R/save_all_plots.R 
  Function: save_all_plots 

Possible missing @noRd for internal helper:
  File: R/table_afsc_tier.R 
  Function: table_afsc_tier 

Possible missing @param docs:
  File: R/table_afsc_tier.R 
  Function: table_afsc_tier 
  Missing: ) {   # REMINDERS: add in code that   # -adds make_rda and tables_dir as arguments   # -defines topic_label, fig_or_table; and   # -makes an rda if make_rda 

Possible missing @noRd for internal helper:
  File: R/table_bnc.R 
  Function: # table_bnc <- function( 

Possible missing @param docs:
  File: R/table_bnc.R 
  Function: # table_bnc <- function( 
  Missing: #     dat, #     end_year, "%Y" 

Possible missing @noRd for internal helper:
  File: R/table_harvest_projection.R 
  Function: table_harvest_projection 

Possible missing @param docs:
  File: R/table_harvest_projection.R 
  Function: table_harvest_projection 
  Missing: ) {   # REMINDERS: add in code that   # -adds make_rda and tables_dir as arguments   # -defines topic_label, fig_or_table; and   # -makes an rda if make_rda 

Possible missing @param docs:
  File: R/table_index.R 
  Function: table_index 
  Missing: table_index <- function(   dat, era, interactive 

Function uses @inheritParams:
  File: R/table_index.R 
  Function: table_index 

Possible missing @param docs:
  File: R/table_landings.R 
  Function: table_landings 
  Missing: table_landings <- function(   dat, unit_label, era, interactive, group 

Function uses @inheritParams:
  File: R/table_landings.R 
  Function: table_landings 

Possible missing @param docs:
  File: R/theme_noaa.R 
  Function: theme_noaa 
  Missing: ...) {   if (utils::packageVersion("ggplot2") < "4.0.0") {     cli::cli_warn(       "Your {.pkg ggplot2} version is {.val {utils::packageVersion('ggplot2')}}, which is older than the version required to use {.fn theme_noaa} (4.0.0 

Possible missing @param docs:
  File: R/theme_table.R 
  Function: theme_table 
  Missing: x) {   if (class(x)[1], quietly, call. 

Possible missing @noRd for internal helper:
  File: R/theme_table.R 
  Function: FitFlextableToPage 

Possible missing @param docs:
  File: R/theme_table.R 
  Function: FitFlextableToPage 
  Missing: ft, pgwidth, width 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: check_year 

Possible missing @param docs:
  File: R/utils.R 
  Function: check_year 
  Missing: end_year, topic, fig_or_table 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: get_ncol 

Possible missing @param docs:
  File: R/utils.R 
  Function: get_ncol 
  Missing: file, skip, quote, comment.char 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: row_match 

Possible missing @param docs:
  File: R/utils.R 
  Function: row_match 
  Missing: x, table, nomatch, "matrix")) table <- as.data.frame(table)   if (is.null(dim(x))) x <- as.data.frame(matrix(x, nrow, c(x[, drop, sep, c(table[, table_concat 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: replace_empty_with_na_all 

Possible missing @param docs:
  File: R/utils.R 
  Function: replace_empty_with_na_all 
  Missing: dat) {   dat <- as.data.frame(dat)   for (col in seq_along(dat)) {     if (is.character(dat[[col]])) {       is_empty <- !is.na(dat[[col]] 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: # SS3_extract_df <- function(dat, label) { 

Possible missing @param docs:
  File: R/utils.R 
  Function: # SS3_extract_df <- function(dat, label) { 
  Missing: row) any(row, return NA #   if (is.na(value_row) 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: SS3_extract_df 

Possible missing @param docs:
  File: R/utils.R 
  Function: SS3_extract_df 
  Missing: dat, label) {   # Convert to data.table if not already   # if (!inherits(dat, "data.table")) {   #   dat <- data.table::as.data.table(dat 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: SS3_extract_fleet 

Possible missing @param docs:
  File: R/utils.R 
  Function: SS3_extract_fleet 
  Missing: dat, vers) {   # Determine where fleet names are located base on model version   # TODO: test other SS3 models and/or write converter based on r4ss::ss_output   vers <- as.numeric(stringr::str_replace(vers, "3.30.", ""))   if (vers < 20.00 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: # baseline_units <- function() { 

Possible missing @noRd for internal helper:
  File: R/utils.R 
  Function: expand_element 

Possible missing @param docs:
  File: R/utils.R 
  Function: expand_element 
  Missing: x, name) {     # Skip empty/null elements immediately     if (length(x) 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: plot_timeseries 
  Missing: plot_timeseries <- function(   dat 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: plot_error 
  Missing: plot_error <- function(   dat, x, y, geom, group 

Function uses @inheritParams:
  File: R/utils_plot.R 
  Function: plot_error 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: plot_aa 
  Missing: plot_aa <- function(   dat, x 

Function uses @inheritParams:
  File: R/utils_plot.R 
  Function: plot_aa 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: average_age_line 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: average_age_line 
  Missing: dat, facet ) {   # Calculate annual mean age   grouping <- intersect(colnames(dat), facet 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: cohort_line 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: cohort_line 
  Missing: dat, x, y, z 

Function uses @inheritParams:
  File: R/utils_plot.R 
  Function: reference_line 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: axis_breaks 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: axis_breaks 
  Missing: data_col) {   # Get the range of the x-axis data (assuming it's a numeric vector)   x_range <- range(data_col, na.rm 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: # y_axis_breaks <- function(data){ 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: # y_axis_breaks <- function(data){ 
  Missing: data){ #   # TODO: generalize this so we can input any column for the y-axis values #   y_n_breaks <- round(length(unique(data[["age"]]))) #   if (y_n_breaks > 80) { #     y_n_breaks <- round(length(unique(data[["age"]])) / 6) #   } else if (y_n_breaks > 40 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: cap_first_letter 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: cap_first_letter 
  Missing: s) {   if (length(s), NA, or zero-length strings   }   first_letter <- toupper(substring(s, 1, 1))   rest_of_string <- substring(s, 2 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: # calculate_uncertainty <- function() { 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: filter_data 
  Missing: filter_data <- function(   dat 

Function uses @inheritParams:
  File: R/utils_plot.R 
  Function: filter_data 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: get_id 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: get_id 
  Missing: dat) {   if (is.null(names(dat))) {     # If the list is unnamed, return the sequence of its elements     return(seq_along(dat) 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: calculate_reference_point 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: calculate_reference_point 
  Missing: dat, reference_name, lbs 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: label_magnitude 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: label_magnitude 
  Missing: label, unit_label, scale_amount, legend 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: check_grouping 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: check_grouping 
  Missing: dat 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: plot_obsvpred 
  Missing: plot_obsvpred <- function(   dat, x, y 

Function uses @inheritParams:
  File: R/utils_plot.R 
  Function: plot_obsvpred 

Possible missing @noRd for internal helper:
  File: R/utils_plot.R 
  Function: add_reference_line 

Possible missing @param docs:
  File: R/utils_plot.R 
  Function: add_reference_line 
  Missing: add_reference_line <- function(   dat, ref_line, label, lbs, scale_amount 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: calc_kqs 
  Missing: calc_kqs <- function(returned_kq 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: fill_in_kqs 
  Missing: ...) {   arg_names <- sapply(substitute(list(...))[-1], deparse)   arg_values <- list(... 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: export_kqs 
  Missing: ...) {   # Open new or existing key quantities csv   if (file.exists(fs::path(getwd(), "key_quantities.csv"))) {     cli::cli_alert_info("Key quantities text file (key_quantities.csv) exists. Newly calculated key quantities will be added to it.", wrap, "key_quantities.csv") 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: insert_kqs 
  Missing: ...) {   if (file.exists(fs::path(getwd(), "captions_alt_text.csv"))) {     cli::cli_alert_info("Captions/alternative text file (captions_alt_text.csv) exists. Newly calculated key quantities will be added to it.", wrap, "captions_alt_text.csv") 

Possible missing @noRd for internal helper:
  File: R/utils_rda.R 
  Function: create_patterns 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: create_patterns 
  Missing: ...) {     # Capture the names from the ellipsis     arg_names <- sapply(rlang::enexprs(...), as.character)      # Get the actual values     vals <- as.character(list(...) 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: create_rda 
  Missing: # REQUIRED: table or plot object to export   topic_label, # REQUIRED   fig_or_table, # REQUIRED   dat, # REQUIRED: only one dat file to base captions and alt text from   dir 

Possible missing @param docs:
  File: R/utils_rda.R 
  Function: extract_caps_alttext 
  Missing: "captions_alt_text.csv" 

Possible missing @noRd for internal helper:
  File: R/utils_table.R 
  Function: check_label_differences 

Possible missing @param docs:
  File: R/utils_table.R 
  Function: check_label_differences 
  Missing: dat, index_variables, id_group, model