Skip to content

Fix plot_biomass and update naming convention for catch in SS3 converter#194

Open
Schiano-NOAA wants to merge 1 commit intomainfrom
fix-biomass-plt
Open

Fix plot_biomass and update naming convention for catch in SS3 converter#194
Schiano-NOAA wants to merge 1 commit intomainfrom
fix-biomass-plt

Conversation

@Schiano-NOAA
Copy link
Collaborator

@Schiano-NOAA Schiano-NOAA commented Feb 23, 2026

Goal:

Fixing a bug identified in #185 which initially (and incorrectly) displayed biomass but it was actually catch. This makes sense since the landings and biomass plots were extremely similar.

What has been changed:

The following fixes are implemented in this PR:

  • Adjust extraction of biomass from the standard output so it only pulls biomass
  • Adjust standard naming conventions in the converter to be accurate to the true quantities/estimates
  • Minor addition to example in table_landings (reviewer can ignore this)

The resulting plot should show biomass over time with error as a cloud if applicable. Another issue was identified when plotting multiple model results. Feel free to test on multiple results, but the primary focus is the accuracy in the estimate plotted. Issues regarding plotting multiple output on the same plot will be addressed in a new PR.

Actions:

To review this PR:

  • Test the convert_output() function on a Report.sso file
  • Test the plot_biomass() function using the example data sett found in the package and converted output in the first bullet
  • Test plot_biomass(dat = list("a"=data1, "b"=data2)) to see if any errors occur when multiple datasets are provided

The maintainers @Schiano-NOAA and @sbreitbart-NOAA will handle any package checks and testing errors that might occur and make adjustments if there are any more issues found after these changes were made. Thanks for contributing the {stockplotr}!

@github-actions
Copy link
Contributor

New version checklist

  • Package version in DESCRIPTION has been updated
  • Release notes have been drafted/published
  • Cheatsheet content has been updated (if applicable)
  • Cheatsheet version has been updated

@github-actions
Copy link
Contributor

Code Metrics Report

Coverage Code to Test Ratio Test Execution Time
71.1% 1:0.1 4m47s

Code coverage of files in pull request scope (92.3%)

Files Coverage
R/plot_biomass.R 90.1%
R/table_landings.R 95.1%

Reported by octocov

@Schiano-NOAA Schiano-NOAA requested review from sbreitbart-NOAA and removed request for sbreitbart-NOAA February 24, 2026 16:33
@Schiano-NOAA
Copy link
Collaborator Author

@Ovec8hkin would you be willing to review this PR? This addressed the issue you opened regarding the plot_biomass function. It was a relatively straight forward fix and should hopefully work well now!

I tried assigning you and adding you to the repository, but your name would not populate. I will try again at another time, but for for the moment, you can either download this branch through

remotes::install_github("nmfs-ost/stockplotr@fix-biomass-plt")

or fork the repository and make adjustments on your own repo then open a PR from that.

Please perform the testing mentioned in the PR description above.

Let me know if you have any questions or are unable to do this at this time 😄

@Ovec8hkin
Copy link

@Schiano-NOAA this appears to work fine for the SPR_SERIES module but still appears to plot landings when module="TIME_SERIES". Looking at the output of the filter_data call, it seems like landings biomass is still getting into the plotting data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants