[Feature]: add option to convert plot units to lbs#237
[Feature]: add option to convert plot units to lbs#237Schiano-NOAA wants to merge 18 commits intomainfrom
Conversation
Schiano-NOAA
changed the title
|
@sbreitbart-NOAA this is ready for review and you will receive a request shortly. I wanted to note first that
We should plan for this to be temporary until the units column is incorporated into the converter for all models. Once that feature is developed, we can adjust the lbs feature in respective plots to pull the units from the conout file and proceed from there (i.e. if units are already lbs and user sets lbs = TRUE then nothing happens and unit_label can be extracted from the model) |
sbreitbart-NOAA
left a comment
sbreitbart-NOAA
left a comment
There was a problem hiding this comment.
Github won't let me comment on individual lines, so I'll add my comments here. Looks like this is working well! Just have a couple of suggestions and a question:
plot_spawning_biomass.R
L39: suggest updating param to something like this:
#' @param lbs A logical value indicating whether to convert the y-axis values, assumed to be in kilograms, to pounds.
L135: Did you end up verifying this, as per your comment?
process_data.R
L13: Same comment as for plot_spawning_biomass.R L39
|
@sbreitbart-NOAA Maybe I should changed assumed to default. We are just setting the default to FALSE saying the output units are in lbs. |
I'm not sure I understand. Could you please rephrase? |
* [Fix] plot_indices (#221) * initial commit of new obs v pred plot * adjust process data so it converts all indexed data to character to function even when input values are numeric per #212 * update plot indices to use new plot_obsvpred function and adjust based on needs * update documentation for package * adjust plot_indices where nfleet = 1 and update test to remove new file produced * add missing dependency * Address #218 by scaling uncertainty intervals if relative = TRUE for B and SB plots * remove previous calcs of relative and add if statement instead * adjust relative=TRUE cases to extract proportion from model rather than calc in fxn * add era into arg for plot_biomass * reflect changes for plotting relative values from the model for spawning_biomass * add other option to find biomass when relative * remove relative option from reference_line * adjust ref line when ref is unfished becomes point * remove option to be relative in F plot * remove commented out code * move placement of unfished reference point in if statement * update documentation * remove outdated tests for sb plot * remove extra test and fix relative testing * remove relative example from sb plt since it doesnt exist in the example data * update documentation * remove rel F quantities --------- Co-authored-by: Sam Schiano <125507018+Schiano-NOAA@users.noreply.github.com>
…n scale label fxn
…m model results as is currently
My apologies. I misunderstood what you said in your first comment. I can add some indication that we assume the default units are kgs |
|
@sbreitbart-NOAA okay thoughts on the new wording? |
So close! I think you're just missing a word ("from"): @param lbs A logical value indicating whether to convert the y-axis values from |
Schiano-NOAA
force-pushed
the
main
branch
2 times, most recently
from
b057594 to
5cabc07
Compare
2 participants
New argument in spawning_biomass, biomass, and landings plot function to convert units from metric tons to lbs
Plan release for v0.11.0