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

Add integration tests for convert_output() #29

Closed
Bai-Li-NOAA opened this issue Aug 27, 2024 · 3 comments · Fixed by #45
Closed

Add integration tests for convert_output() #29

Bai-Li-NOAA opened this issue Aug 27, 2024 · 3 comments · Fixed by #45
Assignees

Comments

@Bai-Li-NOAA
Copy link
Collaborator

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

@Bai-Li-NOAA Bai-Li-NOAA self-assigned this Aug 27, 2024
@Bai-Li-NOAA
Copy link
Collaborator Author

@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:

  • When testing with SS3 model output files, five of them triggered the following warning messages:
Warning messages:
1: In convert_output(output_file = "Report.sso", outdir = all_models[i],  ... :
  There are multiple reported error metrics.
2: In convert_output(output_file = "Report.sso", outdir = all_models[i],  ... :
  There are multiple reported error metrics.
3: In convert_output(output_file = "Report.sso", outdir = all_models[i],  ... :
  There are multiple reported error metrics.
4: In convert_output(output_file = "Report.sso", outdir = all_models[i],  ... :
  There are multiple reported error metrics.
5: In convert_output(output_file = "Report.sso", outdir = all_models[i],  ... :
  There are multiple reported error metrics.

Did I possibly misconfigure any arguments? Or is this behavior expected, and should we be concerned about those error metrics? If not, I'll modify the tests to expect warnings when running convert_output().

  • I noticed that when file_save = TRUE, the formatted object is saved as a CSV in the specified directory but not saved in the environment. Would it be possible to both save the object in the environment and write it to a local file? This way, users could pass the object to other report generation functions while still saving a local copy. Or, was there a specific design reason behind this?

@Schiano-NOAA
Copy link
Collaborator

@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
Copy link
Collaborator Author

@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.

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 a pull request may close this issue.

2 participants