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

Change plot_biomass and spawning_biomass plots to work through errors #34

Merged
merged 3 commits into from
Dec 17, 2024

Conversation

Schiano-NOAA
Copy link
Collaborator

fix(b and sb plots): reformat plots so they still fxn even missing the reference line

Copy link
Collaborator

@sbreitbart-NOAA sbreitbart-NOAA left a comment

Choose a reason for hiding this comment

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

Both fxns seem really flexible and work well under multiple scenarios. I did notice that the spawning biomass plot creates a ref line with an "SB" label, but the biomass plot doesn't have a "B" label. Was that intentional?

@Schiano-NOAA
Copy link
Collaborator Author

Both fxns seem really flexible and work well under multiple scenarios. I did notice that the spawning biomass plot creates a ref line with an "SB" label, but the biomass plot doesn't have a "B" label. Was that intentional?

@sbreitbart-NOAA they are both supposed to have a label when a line is present, but if the value wasn't found in the data set then no line or label should be present. Are you saying that in the example you tested it didn't put a label or that there is no label in the biomass fxn?

@sbreitbart-NOAA
Copy link
Collaborator

Both fxns seem really flexible and work well under multiple scenarios. I did notice that the spawning biomass plot creates a ref line with an "SB" label, but the biomass plot doesn't have a "B" label. Was that intentional?

@sbreitbart-NOAA they are both supposed to have a label when a line is present, but if the value wasn't found in the data set then no line or label should be present. Are you saying that in the example you tested it didn't put a label or that there is no label in the biomass fxn?

In my 2 example datasets, I can get a ref line to show up, but without a label for the biomass plot

@Schiano-NOAA
Copy link
Collaborator Author

Both fxns seem really flexible and work well under multiple scenarios. I did notice that the spawning biomass plot creates a ref line with an "SB" label, but the biomass plot doesn't have a "B" label. Was that intentional?

@sbreitbart-NOAA they are both supposed to have a label when a line is present, but if the value wasn't found in the data set then no line or label should be present. Are you saying that in the example you tested it didn't put a label or that there is no label in the biomass fxn?

In my 2 example datasets, I can get a ref line to show up, but without a label for the biomass plot

@sbreitbart-NOAA Thanks for catching that! Apparently the function never had any notation through this process. I just updated it

@Schiano-NOAA Schiano-NOAA marked this pull request as ready for review December 17, 2024 14:41
R/plot_biomass.R Outdated
@@ -135,7 +135,15 @@ plot_biomass <- function(
y = biomass_label) +
ggplot2::scale_x_continuous(
n.breaks = x_n_breaks,
guide = ggplot2::guide_axis(minor.ticks = TRUE))
guide = ggplot2::guide_axis(minor.ticks = TRUE)) +
+
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two + signs (lines 138, 139) creates an error

Copy link
Collaborator

@sbreitbart-NOAA sbreitbart-NOAA left a comment

Choose a reason for hiding this comment

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

Besides the extra plus sign, looks good!

@Schiano-NOAA Schiano-NOAA merged commit 742c212 into master Dec 17, 2024
8 checks passed
@Schiano-NOAA Schiano-NOAA deleted the fix-b-sb-lines branch December 17, 2024 15:24
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