massbays-tech / MassWateR

R package for working with Massachusetts surface water quality data
https://massbays-tech.github.io/MassWateR
Creative Commons Zero v1.0 Universal
11 stars 3 forks source link

readMWRacc error for three decimals in MDL #59

Closed ben-wetherill closed 1 year ago

ben-wetherill commented 1 year ago

I'm getting an error if there is a value in the MDL or UQL column less than 0.01. File attached. Was this a result of the logic that limited the output decimals in QC Report to 2 decimals? That was for readability, but we don't want to limit the values to 2 decimals.

> accdat <- readMWRacc(dqoaccpth)
Running checks on data quality objectives for accuracy...
    Checking column names... OK
    Checking all required columns are present... OK
Error:  Checking column types...
    Incorrect column type found in columns: MDL-should be numeric

Sample_DQOAccuracy_3decimals.xlsx

fawda123 commented 1 year ago

Oof this is another Excel issue, the three decimal entry for Ammonia gets converted to "1E-3" now that all columns are imported as text. This was previously imported automatically as a number (0.001), but now it gets flagged because of the change to import all columns as text for issue #56 (1E-3). So, it's not because of the readability formatting. Should be fixed now.

ben-wetherill commented 1 year ago

It works now for 0.001, but not for 0.002. See below.

Marcus, are all of these DQOAccuracy errors that we are having (#55, #56, and #59) caused by the change we made in issue #54? It seems that we are fixing things one at a time with special case adjustments. I'm not feeling confident that other issues won't appear in this logic. Issue #54 was not a critical issue. We could go back to the way it was before #54 and simply allow "na" in the value range column. "na" could be understood as another way of saying "all". What do you think? In the end we want the logic to be as simple and robust as possible, without lots of special cases.

Error in exists(cacheKey, where = .rs.WorkingDataEnv, inherits = FALSE) : 
  invalid first argument
Error in assign(cacheKey, frame, .rs.CachedDataEnv) : 
  attempt to use zero-length variable name

Sample_DQOAccuracy_3decimals.xlsx

fawda123 commented 1 year ago

@ben-wetherill the error you're seeing in your previous comment is not related to MassWateR, it's an RStudio bug that now has a fix (https://github.com/rstudio/rstudio/issues/13188).

Yes, that one change in #54 has caused these other cascading issues. If you disregard the last error you're seeing, the imports seem to be working now for these different use cases. I'm comfortable with keeping it as is because I've added appropriate tests to verify these issues are working. But, if we're going the cautious route, it's probably better to revert back to the old behavior.

ben-wetherill commented 1 year ago

Ok. Got it. This looks good now.