Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added units to plots that can produce relative values #49

Merged
merged 8 commits into from
Feb 24, 2025

Conversation

sbreitbart-NOAA
Copy link
Collaborator

NOTE: After writing the text below, I realized that the issue #44 was written confusingly (by me 😄). The "relative" argument is not the same thing as the "scaling" argument. I'll just keep this PR going, as a draft, and turn my attention toward adjusting the alt text to adapt to the scaling argument.


Specifically edited the alt text for relative biomass, recruitment, and spawning biomass as per #44.

I think all that's needed to satisfy that issue is to simply state the units (which weren't stated before in these figures' alt texts; only the captions). The key quantities are being computed correctly, and since the alt text doesn't state the axis units- only the min and max values- I believe it's fine to keep using the regular plots' units for these figures.

For example, this is the alt text for relative recruitment:

Line graph showing the assessment model estimated relative recruitment in sr.units for each year. The x axis shows year, which spans from recruitment.start.year to recruitment.end.year . The y axis shows relative recruitment (R/R0), which spans from rel.recruitment.min to rel.recruitment.max recruitment.units . (I just added the bolded units placeholder in this PR).

If the min and max recruitment are 1,000 and 1,000,000 pounds, then the relative values (R/R0) (could be) 1-1,000 pounds. This alt text would still be sufficient for the relative case:

"The y axis shows relative recruitment (R/R0), which spans from 1 to 1,000 pounds."

@sbreitbart-NOAA sbreitbart-NOAA self-assigned this Feb 21, 2025
@sbreitbart-NOAA sbreitbart-NOAA linked an issue Feb 21, 2025 that may be closed by this pull request
…; updated fxns with this arg (plot_biomass, recruitment, ssb); moved code that computes min/max B, R, ssb from write_captions to add_more_key_quants; updated add_more_key_quants to label scaled values with appropriate label that's inclusive of scale's order of magnitude
…nts into captions/alt text csv; updated exp_all_figs_tables to include new arguments specifying scaling amounts for biomass, recruitment, and sp biomass plots
@sbreitbart-NOAA sbreitbart-NOAA marked this pull request as ready for review February 24, 2025 19:16
Copy link
Collaborator

@Schiano-NOAA Schiano-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good! I have some minor suggestions you can adopt or not. I really like how you identified the scaling and how it will change the units

@sbreitbart-NOAA sbreitbart-NOAA merged commit 4e4833e into master Feb 24, 2025
9 checks passed
Schiano-NOAA pushed a commit that referenced this pull request Feb 24, 2025
* Added units to plots that can produce relative values (biomass, recruitment, and spawning biomass) as per #44

* Added new arg to add_more_key_quants to allow for values to be scaled; updated fxns with this arg (plot_biomass, recruitment, ssb); moved code that computes min/max B, R, ssb from write_captions to add_more_key_quants; updated add_more_key_quants to label scaled values with appropriate label that's inclusive of scale's order of magnitude

* Added code that adds new key quantities extracted in add_more_key_quants into captions/alt text csv; updated exp_all_figs_tables to include new arguments specifying scaling amounts for biomass, recruitment, and sp biomass plots

* Fix test

* Update documentation for exp_all_figs_tables

* Attempt to fix test failures by fixing path in add_more_key_quants()

* add dat arg to B, R, SSB plotting fxns, and add_more_key_quants, to address failing tests

* Updates per Schiano-NOAA's comments on PR (minor edits to code, documentation)
Schiano-NOAA added a commit that referenced this pull request Feb 24, 2025
* change package name

* Added units to plots that can produce relative values (#49)

* Added units to plots that can produce relative values (biomass, recruitment, and spawning biomass) as per #44

* Added new arg to add_more_key_quants to allow for values to be scaled; updated fxns with this arg (plot_biomass, recruitment, ssb); moved code that computes min/max B, R, ssb from write_captions to add_more_key_quants; updated add_more_key_quants to label scaled values with appropriate label that's inclusive of scale's order of magnitude

* Added code that adds new key quantities extracted in add_more_key_quants into captions/alt text csv; updated exp_all_figs_tables to include new arguments specifying scaling amounts for biomass, recruitment, and sp biomass plots

* Fix test

* Update documentation for exp_all_figs_tables

* Attempt to fix test failures by fixing path in add_more_key_quants()

* add dat arg to B, R, SSB plotting fxns, and add_more_key_quants, to address failing tests

* Updates per Schiano-NOAA's comments on PR (minor edits to code, documentation)

* add warning for previous naming

* add callout box for name change

---------

Co-authored-by: Sophie Breitbart <[email protected]>
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.

Alter key quantities code to adapt to scaled plotting argument
2 participants