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 unit tests #40

Merged
merged 25 commits into from
Jan 6, 2025
Merged

Add unit tests #40

merged 25 commits into from
Jan 6, 2025

Conversation

sbreitbart-NOAA
Copy link
Collaborator

Making unit tests for satf

@sbreitbart-NOAA sbreitbart-NOAA changed the title Add tests Add unit tests Dec 31, 2024
… a figure or table so that if one function fails, the rest can still produce rdas
- Fix bug preventing key quantities from being substituted in alt text/captions csv.
- Suppress warnings for write_captions
@sbreitbart-NOAA sbreitbart-NOAA marked this pull request as ready for review January 3, 2025 16:17
R/write_captions.R Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might cause an issue with download times of the package depending on how large this file is. We will need to chat as a group about running a simple SS3 model we can use for a sample data set - for use in both asar and satf

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filtered the dataset to include data collected in 2020 or later, so it's <5 MB. Is that too large?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think under 10 is fine for a package but we'd have to look up the cran standard. I think though have a simple example would be nice because we would be familiar with it and know what to expect

With that said, let's leave this one in for now and revisit

Copy link
Collaborator

@Schiano-NOAA Schiano-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General regarding questions but looks good otherwise

@sbreitbart-NOAA sbreitbart-NOAA merged commit 8ca9ef8 into master Jan 6, 2025
8 checks passed
@sbreitbart-NOAA sbreitbart-NOAA deleted the new-tests branch January 6, 2025 15:42
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 this pull request may close these issues.

2 participants