docs(impute): Update documentation w.r.t. censoredInt and MBimpute#204
docs(impute): Update documentation w.r.t. censoredInt and MBimpute#204tonywu1999 merged 2 commits intodevelfrom
Conversation
📝 WalkthroughWalkthroughDocumentation updates across seven files clarify the semantics of censoring and imputation parameters ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
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 docstrings
🧪 Generate unit tests (beta)
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 |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
man/dot-runTukey.Rd (1)
18-24: Minor: "impute is ignored" text references a parameter.runTukeydoesn't have.
.runTukey()itself has noimputeargument (seeusageon line 7), so the last sentence — which is copy/pasted from the sharedcensored_symbolparagraph used byMSstatsSummarizeSingle*— reads a bit oddly in this helper's Rd page. Also, the'NA' (default)label is inherited wording but the function signature has no default forcensored_symbol. Since this is auto-generated, consider tailoring the roxygen text inR/utils_summarization.Rfor.runTukey(e.g., drop the "(impute is ignored)" clause here, or link todataProcessfor end-user semantics).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@man/dot-runTukey.Rd` around lines 18 - 24, The .runTukey Rd entry incorrectly mentions an impute argument and a default for censored_symbol; update the roxygen in R/utils_summarization.R that generates the .runTukey help so the censored_symbol text for .runTukey does not state "(impute is ignored)" and does not assert "'NA' (default)"; instead provide wording appropriate for .runTukey (e.g., drop the impute clause and omit the default claim or add a note pointing users to dataProcess for semantics), ensuring you change the shared censored_symbol text only for the .runTukey documentation generation and keep MSstatsSummarizeSingle* text unchanged.R/dataProcess.R (1)
191-197: Nit:'NA' (default)label is inaccurate for the exported helpers.
MSstatsSummarizeWithMultipleCores,MSstatsSummarizeWithSingleCore, andMSstatsSummarizeSingleTMPare exported but their signatures don't supply a default forcensored_symbol(onlydataProcess()'scensoredInt = "NA"does). Users calling these helpers directly will get an error, not the "default". Consider rephrasing for these helpers, e.g.'NA' (value used by dataProcess by default)or just'NA'.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/dataProcess.R` around lines 191 - 197, The param doc claims "'NA' (default)" but the exported helper functions MSstatsSummarizeWithMultipleCores, MSstatsSummarizeWithSingleCore, and MSstatsSummarizeSingleTMP do not provide a default for censored_symbol, causing a mismatch; either (A) add censored_symbol = "NA" to those functions' signatures so the doc is accurate, or (B) reword the roxygen `@param` text in R/dataProcess.R to "'NA' (value used by dataProcess by default)" (or just "'NA'") so it no longer asserts a default for the helpers; update the documentation text accordingly and ensure the helper functions' parameter names match exactly (censored_symbol / censoredInt mapping) so the exported help is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@man/dot-runTukey.Rd`:
- Around line 18-24: The .runTukey Rd entry incorrectly mentions an impute
argument and a default for censored_symbol; update the roxygen in
R/utils_summarization.R that generates the .runTukey help so the censored_symbol
text for .runTukey does not state "(impute is ignored)" and does not assert
"'NA' (default)"; instead provide wording appropriate for .runTukey (e.g., drop
the impute clause and omit the default claim or add a note pointing users to
dataProcess for semantics), ensuring you change the shared censored_symbol text
only for the .runTukey documentation generation and keep MSstatsSummarizeSingle*
text unchanged.
In `@R/dataProcess.R`:
- Around line 191-197: The param doc claims "'NA' (default)" but the exported
helper functions MSstatsSummarizeWithMultipleCores,
MSstatsSummarizeWithSingleCore, and MSstatsSummarizeSingleTMP do not provide a
default for censored_symbol, causing a mismatch; either (A) add censored_symbol
= "NA" to those functions' signatures so the doc is accurate, or (B) reword the
roxygen `@param` text in R/dataProcess.R to "'NA' (value used by dataProcess by
default)" (or just "'NA'") so it no longer asserts a default for the helpers;
update the documentation text accordingly and ensure the helper functions'
parameter names match exactly (censored_symbol / censoredInt mapping) so the
exported help is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 74dd8c1f-2323-41df-8cc9-e8faa6642e79
📒 Files selected for processing (7)
R/dataProcess.Rman/MSstatsSummarizeSingleTMP.Rdman/MSstatsSummarizeWithMultipleCores.Rdman/MSstatsSummarizeWithSingleCore.Rdman/dataProcess.Rdman/dot-getNonMissingFilterStats.Rdman/dot-runTukey.Rd
Motivation and Context
Based on this google groups post, it appears the MSstats documentation is not correct w.r.t. how the code handles censoredInt and MBimpute.
PR Type
Documentation
Description
Clarify censored missing-value semantics
NA,0, andNULLDocument imputation behavior consistently
NULLRegenerate matching
.RdreferencesDiagram Walkthrough
File Walkthrough
7 files
Clarify censored-value and imputation parameter docsSync TMP single-protein argument documentationUpdate multicore summarization argument descriptionsUpdate single-core summarization argument descriptionsRefresh `dataProcess` censored and imputation docsClarify internal censored-symbol parameter behaviorAlign Tukey summarization censored-value documentation