From c6bb1b91dc99a375c3ecb4b2d5ff660a39092023 Mon Sep 17 00:00:00 2001 From: sbreitbart-NOAA Date: Tue, 27 Oct 2026 11:12:24 -0400 Subject: [PATCH 1/3] Update extract_sis_data() and vignette with feedback from Jeff --- R/extract_sis_data.R | 25 ++++++------------------- tests/testthat/test-extract_sis_data.R | 4 ++-- vignettes/extract-sis-data.Rmd | 14 +++++++------- 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/R/extract_sis_data.R b/R/extract_sis_data.R index 0e3e3999..44d05303 100644 --- a/R/extract_sis_data.R +++ b/R/extract_sis_data.R @@ -145,29 +145,16 @@ extract_sis_data <- function(sis_data_dir = getwd(), # RECRUITMENT tryCatch({ - load(fs::path(figures_tables_dir, "figures", "abundance_at_age_figure.rda")) |> suppressWarnings() - rec <- rda[["figure"]][["layers"]][["geom_point"]][["data"]] + load(fs::path(figures_tables_dir, "figures", "recruitment_figure.rda")) |> suppressWarnings() + rec <- rda[["figure"]][["layers"]][["geom_line"]][["data"]] recruitment <- rec |> - dplyr::filter(age == 1) |> dplyr::group_by(year) |> - dplyr::summarise(sum = sum(estimate)) |> + dplyr::summarise(sum = sum(predicted_recruitment)) |> dplyr::rename(Recruitment = sum) }, error = function(e) { - cli::cli_alert_warning("Recruitment (age 1) data was not extracted from the 'abundance_at_age_figure.rda' file.") + cli::cli_alert_warning("Recruitment data was not extracted from the 'recruitment_figure.rda' file.") recruitment <<- NULL - - # this is for recruitment in mt- not age-1, which is what SIS expects - # load(fs::path(figures_tables_dir, "figures", "recruitment_figure.rda")) |> suppressWarnings() - # rec <- rda[["figure"]][["layers"]][["geom_line"]]$data - # recruitment <- rec |> - # dplyr::group_by(year) |> - # dplyr::summarise(sum = sum(predicted_recruitment)) |> - # dplyr::rename(Recruitment = sum) - # }, - # error = function(e) { - # cli::cli_alert_warning("Recruitment data was not extracted from the 'recruitment_figure.rda' file.") - # recruitment <<- NULL }) # FISHING MORTALITY @@ -348,7 +335,7 @@ extract_sis_data <- function(sis_data_dir = getwd(), Category == "Spawners" ~ "SSB", Category == "Recruitment" ~ "Recruits - Age 1", Category == "Fmort" ~ "F for Fully-Selected Fish", - Category == "Index" ~ "Index of Abundance", + Category == "Index" ~ "Index", Category == "Catch" ~ "Estimated Total Catch", TRUE ~ NA )) |> @@ -356,7 +343,7 @@ extract_sis_data <- function(sis_data_dir = getwd(), Category == "Biomass" ~ as.character(b_unit), Category == "Spawners" ~ as.character(b_unit), Category == "Fmort" ~ as.character(f_unit), - Category == "Index" ~ "", + Category == "Index" ~ "Index", Category == "Catch" ~ ifelse(kqs$value[kqs$key_quantity == "tot.catch.units"] == " (mt)", "Metric Tons", kqs$value[kqs$key_quantity == "tot.catch.units"]), Category == "Recruitment" ~ "Number of Fish", TRUE ~ NA diff --git a/tests/testthat/test-extract_sis_data.R b/tests/testthat/test-extract_sis_data.R index 45641925..2ddc6555 100644 --- a/tests/testthat/test-extract_sis_data.R +++ b/tests/testthat/test-extract_sis_data.R @@ -11,8 +11,8 @@ test_that("extract_sis_data() runs with all possible rdas present", { # make_rda = TRUE) files <- c( - fs::path(savedir, "figures", - c("biomass_figure.rda", "spawning_biomass_figure.rda", "abundance_at_age_figure.rda", "fishing_mortality_figure.rda", "index_figure.rda") + fs::path("figures", + c("biomass_figure.rda", "spawning_biomass_figure.rda", "recruitment_figure.rda", "fishing_mortality_figure.rda", "index_figure.rda") ), fs::path(savedir, "tables", "total_catch_table.rda") diff --git a/vignettes/extract-sis-data.Rmd b/vignettes/extract-sis-data.Rmd index 2c52b7e0..039b6440 100644 --- a/vignettes/extract-sis-data.Rmd +++ b/vignettes/extract-sis-data.Rmd @@ -33,9 +33,9 @@ This process fits within the following workflow: a. Run `stockplotr::extract_sis_data()` to generate, populate, and export the templates with data originating from a converted model results file. At least one of the following `.rda` files must be present in the `figures/` and/or `tables/` folders: * `biomass_figure.rda` * `spawning_biomass_figure.rda` - * `abundance_at_age_figure.rda` * `fishing_mortality_figure.rda` * `index_figure.rda` + * `recruitment_figure.rda` * `total_catch_table.rda` Ideally, the `key_quantities.csv` file will also be present (in the `key_quantities_dir`). @@ -68,9 +68,9 @@ This file contains metadata and identifiers for the assessment, as well as summa This file is formatted to contain time series data for the stock. The columns are as follows: * **Year**: Observation year. -* **Category**: Time series category. Options include: "Biomass", "Spawners", "Recruitment (age 1)", "Fmort", "Index of Abundance", and "Catch". -* **Primary**: Whether the category is a primary time series category ("Y" or "N"). The following categories can be primary: "Fmort", "Recruitment (age 1)", "Catch", and either "Biomass" OR "Spawners". This function will automatically set the Primary column to "Y" for the applicable time series in each of these categories, and ask the user to confirm which time series should be primary if both "Biomass" and "Spawners" are present. More time series can be added to the template after running `extract_sis_data()`; please see the [Provide additional time series data] section below for more information. -* **Description**: Brief description (e.g., Description for "Fmort" is "F for Fully-Selected Fish"). +* **Category**: Time series category. Options include: "Biomass", "Spawners", "Recruitment", "Fmort", "Index", and "Catch". +* **Primary**: Whether the category is a primary time series category ("Y" or "N"). The following categories can be primary: "Fmort", "Recruitment", "Catch", and either "Biomass" OR "Spawners". This function will automatically set the Primary column to "Y" for the applicable time series in each of these categories, and ask the user to confirm which time series should be primary if both "Biomass" and "Spawners" are present. More time series can be added to the template after running `extract_sis_data()`; please see the [Provide additional time series data] section below for more information. +* **Description**: Brief description that should align with the units in sis_assmt_template.csv (e.g., Description for "Fmort" is "F for Fully-Selected Fish"). This field can/should cross-link to the basis field for Fmort (AS_F_BASIS) and Biomass/Spawners (AS_B_BASIS). * **Unit**: Unit of measurement. * **Value**: Value. @@ -80,9 +80,9 @@ As the main purpose of this function is to automate the extraction of data into * **Biomass**: `stockplotr::plot_biomass()` * **Spawning Biomass**: `stockplotr::plot_spawning_biomass()` -* **Recruitment at Age 1**: `stockplotr::plot_abundance_at_age()` +* **Recruitment**: `stockplotr::plot_recruitment()` * **Fishing Mortality**: `stockplotr::plot_fishing_mortality()` -* **Index of Abundance**: `stockplotr::plot_index()` +* **Index**: `stockplotr::plot_index()` * **Catch**: `stockplotr::table_total_catch()` ## Provide the path to the `key_quantities.csv` file @@ -117,7 +117,7 @@ These values are extracted and inserted into the key_quantities.csv by running ` | F.MSY.terminal.max | AS_FMSY_MAX | | F.MSY.terminal.min | AS_FMSY_MIN | -* Units for the **Index of Abundance** and **Recruitment (age 1)** time series categories are set to *blank* and "Number of Fish", respectively. +* Units for the **Index** and **Recruitment** time series categories are set to "Index" and "Number of Fish", respectively. ## Provide assessment data before or after running `extract_sis_data()` to populate additional fields From 9e38f22260d7ed9af0f41dfc5bec5d88a464ad3d Mon Sep 17 00:00:00 2001 From: sbreitbart-NOAA Date: Tue, 27 Oct 2026 11:21:10 -0400 Subject: [PATCH 2/3] Update vignette to clarify plot rdas can't contain multiple models --- vignettes/extract-sis-data.Rmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vignettes/extract-sis-data.Rmd b/vignettes/extract-sis-data.Rmd index 039b6440..0394d34d 100644 --- a/vignettes/extract-sis-data.Rmd +++ b/vignettes/extract-sis-data.Rmd @@ -37,7 +37,7 @@ This process fits within the following workflow: * `index_figure.rda` * `recruitment_figure.rda` * `total_catch_table.rda` - Ideally, the `key_quantities.csv` file will also be present (in the `key_quantities_dir`). + The exported figures and tables can only contain one model per plot (i.e., an exported figure compare multiple models on the same plot). Ideally, the `key_quantities.csv` file will also be present (in the `key_quantities_dir`). b. Generate blank template files by running `asar::create_blank_sis()`. From 8e397a26f31392569acf21072e3110087a6c21a6 Mon Sep 17 00:00:00 2001 From: sbreitbart-NOAA Date: Thu, 12 Nov 2026 13:53:12 -0500 Subject: [PATCH 3/3] Update extract_sis_data() test --- tests/testthat/test-extract_sis_data.R | 42 +++++++++++++++++++------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/tests/testthat/test-extract_sis_data.R b/tests/testthat/test-extract_sis_data.R index 2ddc6555..46d178bf 100644 --- a/tests/testthat/test-extract_sis_data.R +++ b/tests/testthat/test-extract_sis_data.R @@ -4,14 +4,12 @@ test_that("extract_sis_data() runs with all possible rdas present", { if(!dir.exists(savedir)) dir.create(savedir) - save_all_plots(stockplotr::example_data, figures_tables_dir=savedir) - # table_total_catch(stockplotr::example_data, - # module = "TIME_SERIES", - # interactive = FALSE, - # make_rda = TRUE) + save_all_plots(stockplotr::example_data, + figures_tables_dir = savedir) files <- c( - fs::path("figures", + fs::path(savedir, + "figures", c("biomass_figure.rda", "spawning_biomass_figure.rda", "recruitment_figure.rda", "fishing_mortality_figure.rda", "index_figure.rda") ), fs::path(savedir, "tables", @@ -20,7 +18,13 @@ test_that("extract_sis_data() runs with all possible rdas present", { expect_true(all(file.exists(files))) - extract_sis_data(sis_data_dir=savedir, key_quantities_dir=savedir, figures_tables_dir=savedir) + # remove biomass file to avoid question asking whether to choose + # B or SB as primary + file.remove(fs::path(savedir, "figures", "biomass_figure.rda")) + + extract_sis_data(sis_data_dir = savedir, + key_quantities_dir = savedir, + figures_tables_dir = savedir) # erase temporary testing files unlink(fs::path(savedir), recursive=TRUE) @@ -32,9 +36,21 @@ test_that("extract_sis_data() runs with some of all possible rdas present", { if(!dir.exists(savedir)) dir.create(savedir) - plot_biomass(stockplotr::example_data, module = "TIME_SERIES", interactive = FALSE, make_rda = TRUE, figures_dir=savedir) - plot_fishing_mortality(stockplotr::example_data, interactive = FALSE, make_rda = TRUE, figures_dir=savedir) - expect_no_error(extract_sis_data(sis_data_dir=savedir, key_quantities_dir=savedir, figures_tables_dir=savedir)) + plot_biomass(stockplotr::example_data, + module = "TIME_SERIES", + interactive = FALSE, + make_rda = TRUE, + figures_dir = savedir) + plot_fishing_mortality(stockplotr::example_data, + interactive = FALSE, + make_rda = TRUE, + figures_dir = savedir) + expect_no_error( + extract_sis_data( + sis_data_dir = savedir, + key_quantities_dir = savedir, + figures_tables_dir = savedir) + ) # test that extract_sis_data() exports csvs expect_true(file.exists(fs::path(savedir, "sis_assmt_template.csv"))) @@ -49,7 +65,11 @@ test_that("extract_sis_data() fails with no rdas present", { if(!dir.exists(savedir)) dir.create(savedir) - expect_error(extract_sis_data(sis_data_dir=savedir, key_quantities_dir=savedir, figures_tables_dir=savedir)) + expect_error( + extract_sis_data( + sis_data_dir = savedir, + key_quantities_dir = savedir, + figures_tables_dir = savedir)) # erase temporary testing files unlink(fs::path(savedir), recursive = TRUE)