wqbc icon indicating copy to clipboard operation
wqbc copied to clipboard

`clean_wqdata()` is incorrectly changing detected values to non-detects

Open KarHarker opened this issue 3 years ago • 3 comments

This was previously reported but the error was not fixed by the standardize_mdl_units() rems function https://github.com/bcgov/wqbc/issues/158

I think this is partly happening because the columns change names in standardize_wqdata() and are not recognized in the rems function.

I think the best way to fix might be to:

  • add MDL_UNIT to list of cols kept in standardize_wqdata()
  • add standardize_mdl_units to standardize_wqdata()
  • fix clean_wqdata() function so it does not modify the ResultLetter column

...but open to other thoughts!

Also an issue identified in shinyrems https://github.com/bcgov/shinyrems/issues/123

KarHarker avatar Feb 05 '22 00:02 KarHarker

Hey @KarHarker, I looked into the issue and agree that we should pull the standardize_mdl_units() function into standardize_wqdata(). Since one of the purposes of standardize_wqdata() is to deal with units, it makes the most sense to put standardize_mdl_units() in the standardize function.

I wanted to confirm that this is the general workflow:

# pull ems data
two_year <- rems::get_ems_data(which = "2yr", ask = FALSE)
# filter ems data
data <- rems::filter_ems_data(two_year, emsid = c("0121580"))
# tidy data
data <- wqbc::tidy_ems_data(data)
# standardize data
data <- wqbc::standardize_wqdata(data)
# clean data
data <- wqbc::clean_wqdata(data, by = "EMS_ID")
# calc limit
data <- wqbc::calc_limits(data, by = "EMS_ID", term = "short")

If this is the workflow, then we will need to ensure the wqbc::tidy_ems_data() function keeps the MDL_UNIT column. We can keep the MDL_UNIT column in the output of wqbc::standardize_wqdata() as well. I assume we would not keep the MDL_UNIT column in the output of wqbc::clean_wqdata().

Can you confirm:

  • keep MDL_UNIT column in output table
    • wqbc::tidy_ems_data()
    • wqbc::standardize_wqdata()
  • drop MDL_UNIT column from output table
    • wqbc::clean_wqdata()

Can you clarify what the wqbc::clean_wqdata() function is doing to the ResultLetter column? I see it retains the column in the output but was not able to identify a place where it modified the contents of the column.

aylapear avatar Dec 22 '22 17:12 aylapear

@aylapear I can confirm that is the workflow. Those MDL_UNIT outputs make sense as well. @KarHarker I'm not sure what/if clean_wqdata does the RESULT_LETTER column.

HeatherGranger avatar Dec 22 '22 20:12 HeatherGranger

ResultLetter column should be retained and not have anything done to it given it gives us the information if the Result is less than detection.

HeatherGranger avatar Feb 07 '23 20:02 HeatherGranger