-
Notifications
You must be signed in to change notification settings - Fork 23
Bug fix #768
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
Closed
Closed
Bug fix #768
+183
−120
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Make TADA_FindQCActivities type-stable and consistent with its documentation:
Support the documented “clean” argument options ("none", "all", "duplicates", "blanks", "calibrations", "other") while keeping backward compatibility with logical TRUE/FALSE.
Avoid using .data$ inside dplyr verbs (this shadows dplyr’s pronoun and can cause bugs). Use bare column names or the actual data object name.
Handle NA ActivityTypeCode by assigning “Not Reviewed” (so filtering and messaging work cleanly).
Use message() instead of print() for user-facing informational output.
When flaggedonly = TRUE and there are no QC rows, return a zero-row dataframe with the same schema (slice(flag.data, 0)).
Guard downstream calls (e.g., TADA_CreateComparableID and TADA_OrderCols) to avoid NULL handling issues.
Make TADA_OrderCols safer: Already guarded NULL input and missing ResultIdentifier in your updated version; additionally, use dplyr::arrange when available for clarity. Optionally return a zero-row dataframe with the same schema as input when .data is NULL (if you prefer schema consistency), but your current approach is fine.
The function now always returns a data frame/tibble and never NULL. For zero-row inputs, it adds an empty character column and returns immediately. Character coercion ensures robust concatenation if any of the input columns are factors.
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Preparation
Update your branch from the latest
developand resolve any merge conflictsBefore creating a pull request trigger the format-update GitHub Action on your branch to format the code
Documentation
Add/update inline and/or block comments to clarify complexity, context and intent
Add/update function documentation (roxygen), include working examples, build docs locally using devtools::document(), and inspect added/updated help pages for content and format
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
Maintenance & Data Refresh
Add new dependencies to
DESCRIPTIONand document appropriatelyRun spelling maintenance in
requiredMaintenance.RIf changes affect other package or the shiny app functions, update those impacted functions accordingly
If columns were added/updated, add/update them in
RequiredCols.RRun
.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 refreshTests and checks
Add/update tests in
tests/testthatto cover changesRun
devtools::test()to verify new and existing tests passRun
devtools::check()and address any errors, warnings or notesPull Request Description
Includes a summary of the changes made
Includes relevant context/motivation
Includes links to related issues or pull requests (keywords like "Closes #issue_number" automatically close related issues when pull request is merged)
Review
Review the bot-commented coverage-report generated by test-coverage to confirm all changes are covered by tests
Review/accept any bot-suggested format changes
Request review from at least one developer team member