Fix #185: thread censoredInt correctly and remove spurious unique() in exported script#210
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR refactors missing value handling in the LF summarization pipeline by updating the ChangesSummarization and Data Processing Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Motivation and Context
Issue #185 identified two problems in the MSstatsShiny codebase:
censoredIntparameter was not being properly threaded through theMSstatsHandleMissingfunction call inlf_summarization_loop, which was instead relying on a hardcoded missing value symbolgetDataCode()contained an unnecessaryunique()deduplication step that was producing spurious behavior for non-PTM inputsThis PR fixes both issues by ensuring the censored value indicator from user input is correctly propagated through the data processing pipeline and by removing an extraneous transformation from the generated code.
Changes
R/main_calculations.R
MSstatsHandleMissing()call inlf_summarization_loopto use explicit named parametersmissing_symbolparameter from a hardcoded"NA"value to dynamically useqc_input$censInt, ensuring the censored integer parameter is correctly threaded through the functioncensored_cutoffargument continues to derive fromQC_check(qc_input, loadpage_input)but is now passed as a named parameter for improved code clarityR/utils.R
unique(as.data.frame(data))transformation from thegetDataCode()functionTests
Existing unit tests in
tests/testthat/test-main_calculations.Rcover thelf_summarization_loopfunction, including tests that verify the correct summarization function is called based on the summary method selection. The test suite includes tests for both "linear" and "TMP" summary methods.Tests in
tests/testthat/test-utils.Rvalidate thatgetDataCode()correctly generates character strings for various input configurations and file types.Coding Guidelines
The changes adhere to the existing codebase conventions. The use of explicit named parameters in function calls improves code readability and reduces the risk of parameter order-related bugs.