Skip to content

[feat] Add more key quantities needed for asar SIS exporter function#301

Merged
sbreitbart-NOAA merged 8 commits into
mainfrom
time-kqs
Jul 20, 2026
Merged

[feat] Add more key quantities needed for asar SIS exporter function#301
sbreitbart-NOAA merged 8 commits into
mainfrom
time-kqs

Conversation

@sbreitbart-NOAA

Copy link
Copy Markdown
Collaborator

Add capability to calculate and store key quantities necessary for the in-development asar::export_to_sis() function

@github-actions

Copy link
Copy Markdown
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
Copy Markdown
Contributor

Code Metrics Report

Coverage Code to Test Ratio Test Execution Time
68.1% 1:0.1 8m1s

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

Files Coverage
R/plot_biomass.R 81.7%
R/plot_fishing_mortality.R 94.2%
R/save_all_plots.R 75.5%

Reported by octocov

Comment thread inst/resources/key_quantity_template.csv Outdated
Comment thread inst/resources/key_quantity_template.csv Outdated
Comment thread inst/resources/key_quantity_template.csv Outdated

@Schiano-NOAA Schiano-NOAA left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we can take this out of the function? I think this is making the function much longer than needed and it's not relevant to making the plot since it serves a different purpose.

@sbreitbart-NOAA

Copy link
Copy Markdown
Collaborator Author

Is there any way we can take this out of the function? I think this is making the function much longer than needed and it's not relevant to making the plot since it serves a different purpose.

What code/function are you talking about, specifically? All of it?

@Schiano-NOAA

Copy link
Copy Markdown
Collaborator

@sbreitbart-NOAA any of the key quantity calcs. I think it would be worth it to at least put them in their own function and call them in the plot_x function with the dat as input

@sbreitbart-NOAA

Copy link
Copy Markdown
Collaborator Author

@sbreitbart-NOAA any of the key quantity calcs. I think it would be worth it to at least put them in their own function and call them in the plot_x function with the dat as input

Ok, just made a new function calc_kqs() that calculates the key quantities in a few functions- primarily those that require more than a line to calculate- so the plotting functions aren't as long anymore. Is that sufficient re: your idea?

@Schiano-NOAA

Copy link
Copy Markdown
Collaborator

@sbreitbart-NOAA any of the key quantity calcs. I think it would be worth it to at least put them in their own function and call them in the plot_x function with the dat as input

Ok, just made a new function calc_kqs() that calculates the key quantities in a few functions- primarily those that require more than a line to calculate- so the plotting functions aren't as long anymore. Is that sufficient re: your idea?

Yes this is great. Cleans up the code well and we can still get the quantities we need efficiently 😄

@sbreitbart-NOAA
sbreitbart-NOAA merged commit 3237615 into main Jul 20, 2026
8 checks passed
@sbreitbart-NOAA
sbreitbart-NOAA deleted the time-kqs branch July 20, 2026 18:01
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