[feat] catch table for exec summary - #350
Conversation
New version checklist
|
Code Metrics Report
Code coverage of files in pull request scope (81.4%)
Reported by octocov |
| report.version.model.changes,table,Document version history briefly describing when major changes or updates are made to each version of the report., | ||
| projections,table,"Forecasted catch, spawning biomass, and fishing mortality over future years. ", | ||
| catch,table,Observed (points) and model estimated (line) catch over time for fleet catch.fleet., | ||
| catch,table,Total catch over time., |
There was a problem hiding this comment.
| catch,table,Total catch over time., | |
| tot.catch,table,Total catch over time., |
There was a problem hiding this comment.
After testing, I noticed this table is showing the results by fleet, so I think we need to include that still?
| # Filter data for catch | ||
| prepared_data <- filter_data( | ||
| dat = dat, | ||
| label_name = "catch", |
There was a problem hiding this comment.
| label_name = "catch", | |
| label_name = "^catch", |
Do you know if there's any reason we can't do this? I think it would reduce the number of module options in the first step
There was a problem hiding this comment.
Why do you need to do this?
Schiano-NOAA
left a comment
Schiano-NOAA
left a comment
There was a problem hiding this comment.
I think this overall is great! I made a few minor suggestions. Could you please change the table name to "table_total_catch" to add clarity. This seems to work as expected on testing so glad for that! There was some inconsistency in the caption vs what was returned which I mentioned in a comment. Once these things are changed and lables/test/everything else are aligned with these edits, then should be good to merge
2 participants
Table showing catch, for the executive summary
@Schiano-NOAA I didn't use the typical process_table() step because it didn't seem to fit with this pipeline. Also made this suitable for creating one table at a time