[feat]: SPR Timeseries

[JZahner1]

plot_spr allows for plotting a timeseries of spawning potential ratio (SPR) quantities.

Users may specify one of three options to customize what quantity is plotted:

• spr - spawning potential ratio
• fishing_intensity - fishing intensity, or 1-SPR
• spr_ratio - the quantity associated with the SPR derived quantity. For some model (e.g., SS3), this quantity may be customizable by the user.

Some models may only plot spr_ratio with uncertainty intervals.

[JZahner1]

@Schiano-NOAA Do we want to add SPR tables to this PR?

[sbreitbart-NOAA]

Thanks for working on this, @JZahner1! I've added some comments about how to update the section where the plot is exported. I'll let @Schiano-NOAA review the rest of the plot. Happy to answer questions here or in a meeting if that's easier.

[Schiano-NOAA]

@Schiano-NOAA Do we want to add SPR tables to this PR?

@JZahner1 Just the plot please! If you want to make a table, it should be in a separate PR however I am not sure that's on our planned development list.

[Schiano-NOAA]

please adjust this so there is some default selection

Suggested change

	quantity <- match.arg(quantity)
	if (length(quantity) > 1 quantity <- "spr" else quantity <- match.arg(quantity)

[Schiano-NOAA]

or alternative put a warning that there is no selected quantity so when it fails if the user doesn't put something in there, they know what is going on

[JZahner1]

@Schiano-NOAA Im pretty sure that R defaults to using the first element if a vector is set as a default parameter value. So if no value is provided, "spr" is used. I can certainly add that to the function documentation. Restricting multiple selections is a good idea though.

[Schiano-NOAA]

Ah okay i was wondering if it did. I was testing it in line and and it didn't select one in the swtich but might behave differently within a function

[JZahner1]

yah. match.arg doesn't really work outside of a function

[kellijohnson-NOAA]

match.arg will take the first element of the vector provided in the function call specifications as the default. There is an internal argument to match.arg to not allow multiple selections, i.e., several.ok = FALSE.

[JZahner1]

@kellijohnson-NOAA thanks for sharing! Does it throw an error if multiple options are provided but several.ok=FALSE or does it default back to the first valid option?

[kellijohnson-NOAA]

It errors

calculate_stat <- function(x, type = c("mean", "median", "sd")) {
      # 1. Validate the user input against the default vector options above
      type <- match.arg(type, several.ok = FALSE)
      
      # 2. Execute code based on the validated choice
      switch(type,
             mean   = mean(x, na.rm = TRUE),
             median = median(x, na.rm = TRUE),
             sd     = sd(x, na.rm = TRUE))
    }

calculate_stat(1:10)
[1] 5.5

calculate_stat(1:10, type = c("mean", "median"))
Error in match.arg(type, several.ok = FALSE) : 'arg' must be of length 1

[JZahner1]

@sbreitbart-NOAA @Schiano-NOAA is this behavior we want (e.g., throwing an error when multiple options are input), or do we want to fallback to the default "spr" quantity? Both seem equally reasonable to me.

[Schiano-NOAA]

Hmm tough call. I think it would be good to fall back to the default and provide a warning message that it has > 1 selected so setting to default or you could set it to the first option provided.

[Schiano-NOAA]

I don't see scale_amount set as an argument in the function and I don't think we want it?

Suggested change

	scale_amount = scale_amount
	scale_amount = 1

[Schiano-NOAA]

This stopped me from creating an rda

[sbreitbart-NOAA]

I originally commented (made a code "suggestion") where I tried to "suggest" removing the unit_label line. It may have gotten wonky and erased the scale_amount line. In any case, to be clear, the scale_amount line was originally = scale_amount and should remain that way (unless I'm missing something, @Schiano-NOAA ). The unit_label line can be deleted. So, it should look like this:

      dat = dat,
      dir = figures_dir,
      scale_amount = scale_amount
    )
  }

[JZahner1]

scale_amount isn't defined as a function input (i guess you can pass it as an extra parameter), so I dont think we can use do scale_amount=scale_amount here. Plus, for SPR, there are no units, so theres nothing to "scale" anyway.

[Schiano-NOAA]

Yeah it should be changed to 1 as Josh did ⬆️

[Schiano-NOAA]

This is great! 🥳 Thank you for resolving the issues as I reviewed. Once all comments are addressed/resolved, I will merge the PR in. Thank you so much for your work on this.

Edit: see other comment about BAM

[Schiano-NOAA]

My apologies there is one more thing. This is not functional with BAM output. BAM contains "spr" so I am not sure why it's not working. I will send you an example file to test and work on