update definition of sb based on reviewer comments #50
Conversation
R/SpawningBiomass.R
Outdated
| #' environmental conditions. | ||
| #' The mass of fish (males and females or females only) in the | ||
| #' population that contribute to reproduction. Often conventionally | ||
| #' defined as the product of weight-at-age and the proportion |
There was a problem hiding this comment.
weight-at-age should be "weight at age"
R/SpawningBiomass.R
Outdated
| #' The mass of fish (males and females or females only) in the | ||
| #' population that contribute to reproduction. Often conventionally | ||
| #' defined as the product of weight-at-age and the proportion | ||
| #' mature-at-age. Alternatively, it can be defined as the biomass of |
There was a problem hiding this comment.
mature-at-age should be mature at age
R/SpawningBiomass.R
Outdated
| #' defined as the product of weight-at-age and the proportion | ||
| #' mature-at-age. Alternatively, it can be defined as the biomass of | ||
| #' all individuals at or above “age at 50 percent maturity” or “size | ||
| #' at 50 percent maturity.” or the total biomass of fish of reproductive |
There was a problem hiding this comment.
remove the full stop after maturity
R/SpawningBiomass.R
Outdated
| #' mature-at-age. Alternatively, it can be defined as the biomass of | ||
| #' all individuals at or above “age at 50 percent maturity” or “size | ||
| #' at 50 percent maturity.” or the total biomass of fish of reproductive | ||
| #' age during the breeding season of a stock. Most often used as a proxy |
There was a problem hiding this comment.
Remove "of a stock"
There was a problem hiding this comment.
That is our bad, since it was in the original text too. I agree that this should be revised.
R/SpawningBiomass.R
Outdated
| #' at 50 percent maturity.” or the total biomass of fish of reproductive | ||
| #' age during the breeding season of a stock. Most often used as a proxy | ||
| #' for measuring egg production, the spawning biomass depends on the | ||
| #' abundance of the various age classes composing the stock and their |
There was a problem hiding this comment.
change stock to population
R/SpawningBiomass.R
Outdated
| #' all individuals at or above “age at 50 percent maturity” or “size | ||
| #' at 50 percent maturity.” or the total biomass of fish of reproductive | ||
| #' age during the breeding season of a stock. Most often used as a proxy | ||
| #' for measuring egg production, the spawning biomass depends on the |
There was a problem hiding this comment.
"Most often used as a proxy for measuring egg production" does not seem to be tied to the rest of the sentence 🤷 . With the comma after the clause it appears to be a qualifying statement but I am not seeing the connection. Perhaps make it its own sentence and put it at the end of the paragraph.
man/Catch.Rd
Outdated
There was a problem hiding this comment.
I am guessing that this change in Catch.Rd is a byproduct of some other commit without the documentation being rendered? Perhaps we should move it into its own commit?
1. Minor text edits 2. Remove the word stock and replace with population 3. Revise size at first maturity to 50% 4. Add definition that is can be the product of weight at age and proportion mature at age
chantelwetzel-noaa
force-pushed
the
update_sb
branch
from
f16705e to
925a03c
Compare
|
@kellijohnson-NOAA I have separated out the catch edits to its own commit and have made the suggested edits for spawning biomass. |
|
@Bai-Li-NOAA I think me question to you in this review is about the Shiny page. Should the process to be to publish the revised webpage post-merging in the pull request or does that happen automatically with the github actions? Thank you |
|
@chantelwetzel-noaa, this is embarrassing, I don't remember how it works. @ChristineStawitz-NOAA might be able to provide a better answer to this question. |
|
@chantelwetzel-noaa and @Bai-Li-NOAA , sorry for lurking, but I think I can help a bit with this one! I just gave both of you edit permissions on the Shiny app on Posit connect. It looks like it was published to deploy from a branch called shiny_app and to check automatically for updates on the branch. However, I think the branch no longer exists, so it can't pull in updates. Therefore, it may need to be re-deployed from an existing branch of your choosing, if you would like it to stay automatically up to date. One note is that the manifest file may need to be updated periodically (either manually and pushed to the branch, or through a github action). I didn't look at the ghas in this repo, so apologize if this is already being dealt with! |
|
@k-doering-NOAA You are the kind of lurker that everyone loves. Thank you for taking the time to look at this. I'll see If I can get the set back up based on your suggestions. |
Revised the definition of SB based on reviewer comments.