USEPA / EPATADA

This R package can be used to compile and evaluate Water Quality Portal (WQP) data for samples collected from surface water monitoring sites on streams and lakes. It can be used to create applications that support water quality programs and help states, tribes, and other stakeholders efficiently analyze the data.
https://usepa.github.io/EPATADA/
Creative Commons Zero v1.0 Universal
40 stars 18 forks source link

Improve test for TADA_PairForCriteriaCalc #525

Closed hillarymarler closed 3 weeks ago

hillarymarler commented 1 month ago

In test-CriteriaComparison.R, test occasionally fails when the random test df does not contain any of the characteristics (pH, temp, salinity, chloride) to be paired. Test needs to be updated to re-run TADA_RandomTestingData until a data set containing one or more of the paired characteristics is found.

wokenny13 commented 1 month ago

@hillarymarler @cristinamullin

Would it make sense for test_that() in test-CriteriaComparison.R to contain any paired parameter argument (with TADA_DataRetrieval() ) options rather than run TADA_RandomTestingData() for testdat1?

Or would we want to continually re-run the TADA_RandomTestingData for a particular reason when running a test function?

wokenny13 commented 1 month ago

I included tryCatch() and try() base functions to handle rerunning TADA_RandomTestingData() for testdat1 if they do not contain any paired parameters needed to run TADA_PairForCriteriaCalc. These changes have been made within the WQXunitRef pull request.

The test_that will attempt to run up to 10 times in a for loop (can make this iteration larger, but if the dataset does not include the paired parameter after a certain amount of times, this may be an external/internal error that may be outside of the CriteriaCalc functions and will slow down the check process if the loop runs for too long).

Reviewing and testing of this logic should be done to ensure it works as intended.

hillarymarler commented 1 month ago

That sounds like a great solution! Is PR #522 ready for review or are there still additional changes you would like to make first?

wokenny13 commented 1 month ago

That sounds like a great solution! Is PR #522 ready for review or are there still additional changes you would like to make first?

522 is ready for review