issue #633 - #979
Conversation
for more information, see https://pre-commit.ci
|
coverage-report
Minimum allowed coverage is Generated by 🐒 cobertura-action against cef2678 |
cristinamullin
left a comment
There was a problem hiding this comment.
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!
for more information, see https://pre-commit.ci
|
I added a flag type called "No unit associated with result value" and added two tests for this flag type |
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
developand resolve any merge conflictsRun devtools::test(), devtools::check(), and devtools::document() locally; ensure tests pass and fix any errors, warnings, or notes. Add new dependencies to
DESCRIPTIONand document appropriatelyAdd/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 coveredConditional
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 viaMaintenanceScheduled.Ror trigger the Component File Update GitHub ActionIf new example data files were added, document them in
ExampleData.Rand include them inMaintenanceScheduled.Rfor regular refreshIf columns were added/updated, update
RequiredCols.RIf changes affect other package or the shiny app functions, update those impacted functions accordingly