nmfs-ost / asar

Partially automate a U.S. stock assessment report.
https://nmfs-ost.github.io/asar/
MIT License
15 stars 3 forks source link

Add integration tests for `convert_output()` #29

Closed Bai-Li-NOAA closed 2 months ago

Bai-Li-NOAA commented 3 months ago

Ensure that the function works with a set of input files and returns standardized assessment model results

Bai-Li-NOAA commented 2 months ago

@Schiano-NOAA, I've added integration tests for convert_output() using the SS3 test models (see test-convert-output branch). These tests are set to run on three operating systems. I have a couple of questions before merging the changes into main:

Schiano-NOAA commented 2 months ago

@Bai-Li-NOAA I made the warning to let the user know about some minor changes in the conversion process. These are nothing to worry about and I will most likely make changes to this in the future.

For the file_save = TRUE. My intention was that if file_save = TRUE then the user will be exporting it as a csv otherwise when file_save = FALSE, the user can name the function then save it as an object to their global environment such as

output <- convert_output(file_save = FALSE)

Setting it up this way allows the user either option. I could just set it up so it does both, but I feel like giving the user the option to save as an object allows them to manipulate it and then the user could just save it as a csv themself after the fact if they want to.

Bai-Li-NOAA commented 2 months ago

@Schiano-NOAA, great! I've modified the tests so that warnings no longer cause a test failure. As for the file_save flag, the original setting works perfectly. My bad, I made a mistake on my end and ended up with NULL when running output <- convert_output(file_save = FALSE). Will submit a pull request once all the tests pass.