issue #633

[ferribyh]

updating TADA_ConvertSpecialChars() so that records with missing or blank result units are removed when clean = TRUE and ResultMeasureValue is being converted

Changes:
-Added a check in TADA_ConvertSpecialChars() to remove rows where TADA.ResultMeasure.MeasureUnitCode is missing or blank during ResultMeasureValue conversion when clean = TRUE
-Limited the new filtering behavior to the ResultMeasureValue workflow so that conversions of other columns are unaffected
-Added a unit test covering the new cleaning behavior

Pull Request Checklist (convert PR to draft if in progress)

Required

• Update your branch from the latest develop and resolve any merge conflicts

• Run devtools::test(), devtools::check(), and devtools::document() locally; ensure tests pass and fix any errors, warnings, or notes. Add new dependencies to DESCRIPTION and document appropriately

• Add/update vignettes for corresponding changes in functionality, list these under articles in _pkgdown.yml, and ensure added/updated vignettes run and build with proper formatting locally

• Request review from at least one developer team member (convert PR to ready for review if it was designated as in progress)

Best practices

• Include a summary of the changes made and relevant context/motivation

• Link issues to auto-close on merge (use Development sidebar or include "Closes #" in the PR)

• Refresh inline/block comments for clarity

• Update roxygen docs and include examples; review help pages

• Add/update tests in tests/testthat; review the bot's coverage report from test-coverage and confirm all changes are covered

Conditional

• If there is a bot spelling comment, run spelling::spell_check_package() locally and fix any misspellings; add approved project terms to WORDLIST with spelling::update_wordlist()

• If tests fail suggesting internal reference files need a refresh, run .TADA_UpdateRefFiles() and .TADA_UpdateExampleData() locally via MaintenanceScheduled.R or trigger the Component File Update GitHub Action

• If new example data files were added, document them in ExampleData.R and include them in MaintenanceScheduled.R for regular refresh

• If columns were added/updated, update RequiredCols.R

• If changes affect other package or the shiny app functions, update those impacted functions accordingly

[github-actions]

coverage-report

File	Coverage		Missing
All files	67%	✅
R/Utilities.R	70%	✅	562 680-681 685 690-692 787 1100-1102 1170-1171 1225-1229 1339-1340 1344-1345 1355-1359 1364-1369 1411-1566 1657 1713 1721 1781 1789-1809 1835-1836 1845-1859 1883-1886 1950-1951 1968-1970 1975 1978-1980 2064-2142 2173-2175 2211-2379 2406-2410 2419-2423 2640 2693 2699-2702 2706 2717 2736-2737 2739-2754 2756 2758 2764 2767-2769 2771 2806 2821 2827 2835 2840 2848-2853 2882 2892-2894 2915-2919 2929-2931 2934-2939 2943-2948 2956 2966-2967 2978 2994 3026-3060 3175 3184 3223-3227

Minimum allowed coverage is 20%

Generated by 🐒 cobertura-action against 34e3d9d

[cristinamullin]

Rows with numeric Result Values but NA units should be flagged accordingly when clean = F.

Currently, it looks like the TADA.ResultMeasureValueDataTypes.Flag will show it as 'numeric'. and when clean = F. Those those rows are kept which is good, but the flag is not detailed enough. When clean = T that row will get removed which is the expected behavior.

Can you please update the flag when clean = F for rows that have blank units to be "No unit associated with result value"? That way we can leverage it for tracking row removal reasons in TADAShiny.

Thanks!

[ferribyh]

I added a flag type called "No unit associated with result value" and added two tests for this flag type

[wokenny13]

when clean = TRUE, should those rows labeled as "No unit associated with result value" still be kept? The flag seems to work as expected for rows with numeric Result Values but NA units "No unit associated with result value"

Those rows flagged as "No unit associated with result value" are kept when clean = T as shown below.
`testdat3 <- TADA_RandomTestingData()

testdat4True <- TADA_ConvertSpecialChars(testdat3,
col = "TADA.ResultMeasureValue",
clean = TRUE)

filteredTrue <- testdat4True |> dplyr::select(ResultMeasureValue, ResultMeasure.MeasureUnitCode, TADA.ResultMeasureValue, TADA.ResultMeasure.MeasureUnitCode, TADA.ResultMeasureValueDataTypes.Flag)`

[wokenny13]

@cristinamullin WEATHER CONDITION (WMO CODE 4501) (CHOICE LIST) has a blank ResultMeasure.MeasureUnitCode that gets converted to "NONE" and remains kept in the data frame when clean = T. I will run some other test df to see if other characteristic names also contain a blank or NA ResultMeasureUnit.Code with ResultMeasureValue populated.

[wokenny13]

It looks like any characteristics with a blank ResultMeasure.MeasureUnitCode gets converted to NONE as the TADA.ResultMeasure.MeasureUnitCode.

Should there be a separate flag value for cases in which

1. TADA.ResultMeasureValue is numeric, but the TADA.ResultMeasure.MeasureUnitCode is NONE
2. TADA.ResultMeasureValue is not numeric and the TADA.ResultMeasure.MeasureUnitCode is NONE

Both cases above seems to get labeled as "No unit associated with result value". Should case number 2 show other flag values?

For case 1, when clean = T, these observations should be kept is that correct? With cases like PH it would make sense to keep them which I think it already currently does. But for case 2 those should be removed when clean = T. Having a different flag value for case 1 vs 2 would make the most sense.

[wokenny13]

@cristinamullin when using the argument input for col = "TADA.ResultMeasureValue" vs col = "ResultMeasureValue" I noticed that when using ResultMeasureValue it leaves non PH char with no units as blanks for TADA.ResultMeasure.MeasureUnitCode, but if you use TADA.ResultMeasureValue it converts those non PH char with no units to "NONE" for TADA.ResultMeasure.MeasureUnitCode. Is there any reason to expect this output? Which input is expected or is both arg input for with/without the TADA prefix supposed to result in the same output?

[wokenny13]

PR is ready for review @cristinamullin

1. Users will get a stop error message if they try to run TADA_ConvertSpecialChars with a TADA prefix column that already exists. For example, a user who has ran autoclean on a function would get an error message if they try to supply arg input col = "ResultMeasureValue". If they truly want to run it on that column again, they can rename the already existing "TADA.ResultMeasureValue" to another name such as "TADA.ResultMeasureValue.Copy"

2. the flag is now "No unit associated with measure value" only for instances in which the resultmeasurevalue or detectionquantitationlimitmeasurevalue is numeric but the units for it are NA, "NONE", "" or " "

3. clean = T will now keep "No unit associated with measure value"

4. updated tests

5. Note: running TADA_ConvertSpecialChars with col= "ResultMeasureValue" when TADA.ResultMeasure.MeasureUnitCode does not exist yet, will create the TADA.ResultMeasure.MeasureUnitCode and just capitalize it to flag it accordingly (as well as handle any DO % ranges correctly). No other conversions of it is being done yet, which follows the logic done in autoclean I believe. If there is any desire to convert the units at this step it could be considered, see code below.

testdat <- Data_R5_TADAPackageDemo[1:4, ]

  testdat$ResultMeasureValue <- c("1.2", "2.3", "3.4", "4.5")
  testdat$ResultMeasure.MeasureUnitCode <- c("mg/L", NA_character_, "", "ug/L")

  result <- TADA_ConvertSpecialChars(
    testdat,
    col = "ResultMeasureValue",
    clean = TRUE
  ) |>
    dplyr::arrange(ResultMeasureValue)

 setdiff(names(result), names(testdat))