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

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

Open
Tracked by #167
KarHarker opened this issue Feb 5, 2022 · 3 comments
Open
Tracked by #167

Comments

@KarHarker
Copy link
Contributor

KarHarker commented Feb 5, 2022

This was previously reported but the error was not fixed by the standardize_mdl_units() rems function #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 bcgov/shinyrems#123

@KarHarker KarHarker changed the title clean_wqdata() is incorrectly changing detected values to non-detects clean_wqdata() is incorrectly changing detected values to non-detects Feb 9, 2022
@aylapear
Copy link
Contributor

aylapear commented Dec 22, 2022

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.

@HeatherGranger
Copy link
Contributor

@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
Copy link
Contributor

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.

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

No branches or pull requests

3 participants