Add convert_output from {asar} #167
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the convert_output() function from the {asar} package to {stockplotr}, reflecting that the function is more appropriate for this package given dependencies. The migration includes the function code, documentation, tests, vignette, and supporting naming convention files for SS3, BAM, and FIMS models.
Key Changes:
- Added
convert_output()function to standardize assessment model output across different formats - Included comprehensive vignette explaining the standardization process and naming conventions
- Added test suite and fixture preparation scripts for SS3 models
- Added CSV files with standard naming conventions for SS3, BAM, and FIMS models
Reviewed changes
Copilot reviewed 11 out of 31 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| R/convert_output.R | Main function implementation for converting assessment model outputs to standardized format |
| tests/testthat/test-convert_output.R | Test suite for convert_output function with SS3 model tests |
| tests/testthat/fixtures/prepare_ss3_output_files.R | Test fixture preparation script for downloading and running SS3 models |
| vignettes/convert_output_details.Rmd | Comprehensive documentation of standardization process and naming conventions |
| man/convert_output.Rd | Generated documentation for convert_output function |
| man/stockplotr-package.Rd | Updated package documentation with r-universe link |
| inst/resources/ss3_var_names.csv | SS3 variable name standardization mappings |
| inst/resources/fims_var_names.csv | FIMS variable name standardization mappings |
| inst/resources/bam_var_names.csv | BAM variable name standardization mappings |
| pkgdown/_pkgdown.yml | Added vignette link to package website navigation |
| NAMESPACE | Added convert_output to exported functions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Schiano-NOAA
force-pushed
the
conout-to-stockplotr
branch
from
075aada to
58eb93f
Compare
Contributor
New version checklist
|
Contributor
New version checklist
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
convert_output()was a previous export from asar. Over time, we have found that the functions in this package are more dependent on the converted output and would fit better in {stockplotr} than {asar}. The files from {asar} have been moved from asar to this repo including:References to this function within asar or stockplotr have been changed (i.e.
asar::convert_output()-->stockplotr::convert_output()What to know