Skip to content

Fix docs#136

Merged
tonywu1999 merged 2 commits into
develfrom
fix-docs
May 27, 2026
Merged

Fix docs#136
tonywu1999 merged 2 commits into
develfrom
fix-docs

Conversation

@tonywu1999
Copy link
Copy Markdown
Contributor

@tonywu1999 tonywu1999 commented May 27, 2026

Motivation and Context

This PR addresses documentation inconsistencies and clarity issues in the MSstatsConvert package, particularly around the summaryforMultipleRows parameter used across multiple converter functions. Users were confused about the scope of this parameter—specifically, whether it controlled aggregation across fractions of the same biological mixture. The PR clarifies that this parameter only applies to handling duplicate PSMs (peptide-spectrum matches) within a single MS run, and explicitly states that it does NOT control collapsing across fractions. The PR also updates the roxygen2 configuration and improves the vignette explanation of fraction handling.

Detailed Changes

Package Metadata

  • Updated DESCRIPTION file: removed RoxygenNote: 7.3.3 field and added Config/roxygen2/version: 8.0.0 to reflect roxygen2 configuration version change
  • Added Anthony Wu (wu.anthon@northeastern.edu) to the package authors list in man/MSstatsConvert.Rd

Documentation Parameter Clarifications

Expanded the summaryforMultipleRows argument documentation across 19 converter function man pages to consistently clarify:

  • The parameter applies to duplicate PSMs within a single MS run (not across fractions)
  • Behavior options: either use the highest intensity (max) or sum of duplicate intensities
  • Default behavior differs by converter type: max for label-free converters, sum for TMT converters
  • Explicit statement that the parameter does not control collapsing across fractions of the same biological mixture

Affected functions:

  • DIAUmpiretoMSstatsFormat
  • FragPipetoMSstatsFormat
  • MaxQtoMSstatsFormat
  • MaxQtoMSstatsTMTFormat
  • MetamorpheusToMSstatsFormat
  • OpenMStoMSstatsFormat
  • OpenSWATHtoMSstatsFormat
  • PDtoMSstatsFormat
  • PDtoMSstatsTMTFormat
  • PhilosophertoMSstatsTMTFormat
  • ProgenesistoMSstatsFormat
  • ProteinProspectortoMSstatsTMTFormat
  • SpectroMinetoMSstatsTMTFormat
  • SpectronauttoMSstatsFormat
  • .sharedParametersAmongConverters (internal helper)

Internal Documentation Updates

  • Updated roxygen documentation in R/utils_documentation.R for the .sharedParametersAmongConverters() function with the same parameter clarifications
  • Updated .getFullDesign.Rd man page with minor formatting adjustments (reordered and reformatted feature_col and measurement_col argument entries)

Vignette Updates

  • Rewrote the "Fractions and balanced design" section in vignettes/msstats_data_format.Rmd to:
    • Explicitly describe how MSstatsBalancedDesign selects a single fraction for overlapped features in label-free/SRM data (using largest number of non-missing MS runs; ties broken by highest mean intensity)
    • Clarify how rows are adjusted to ensure each feature has entries for every run/channel within selected fractions
    • Document how missing intensities are represented as NA
    • Add clarification distinguishing fraction-collapsing logic from the summaryforMultipleRows converter parameter

Unit Tests

No unit tests were added or modified in this pull request. This is a documentation-only release.

Coding Guidelines

According to the project's CONTRIBUTING.md guidelines, all changes appropriately:

  • Modified roxygen2 documentation in .R files rather than directly editing .Rd files (following contribution guidelines for documentation updates)
  • Used roxygen2 and R Markdown for documentation as specified in the project standards
  • Maintained consistency with the existing documentation style across all converter functions

Note: The PR does not appear to include updates to NEWS.md or package version bump, which are typically required according to the contribution guidelines before merging. These items may need to be addressed before final approval.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

This PR updates package documentation and metadata for MSstatsConvert. The main change clarifies the summaryforMultipleRows parameter behavior across 16 converter functions by explicitly describing how it handles duplicate PSMs within single MS runs and distinguishing it from fraction-collapsing logic. Roxygen2 configuration is upgraded, author metadata is added, and the vignette is expanded for clarity.

Changes

Documentation and Metadata Clarification

Layer / File(s) Summary
Roxygen2 configuration upgrade
DESCRIPTION
Updated package metadata to replace RoxygenNote: 7.3.3 with Config/roxygen2/version: 8.0.0, pinning roxygen2 version via explicit configuration field.
summaryforMultipleRows parameter documentation clarification
R/utils_documentation.R, man/dot-sharedParametersAmongConverters.Rd, man/DIAUmpiretoMSstatsFormat.Rd, man/FragPipetoMSstatsFormat.Rd, man/MaxQtoMSstatsFormat.Rd, man/MaxQtoMSstatsTMTFormat.Rd, man/MetamorpheusToMSstatsFormat.Rd, man/OpenMStoMSstatsFormat.Rd, man/OpenSWATHtoMSstatsFormat.Rd, man/PDtoMSstatsFormat.Rd, man/PDtoMSstatsTMTFormat.Rd, man/PhilosophertoMSstatsTMTFormat.Rd, man/ProgenesistoMSstatsFormat.Rd, man/ProteinProspectortoMSstatsTMTFormat.Rd, man/SpectroMinetoMSstatsTMTFormat.Rd, man/SpectronauttoMSstatsFormat.Rd
Expanded parameter descriptions across source documentation and 16 converter function help pages to clarify that summaryforMultipleRows aggregates duplicate PSMs (identical features identified by multiple PSMs within a single MS run) by selecting max or sum of their intensities. Defaults are label-free converters (max) vs TMT converters (sum). All descriptions now explicitly note that this parameter does not control collapsing of the same feature across fractions of the same biological mixture.
Author metadata and additional documentation updates
man/MSstatsConvert.Rd, man/dot-getFullDesign.Rd, vignettes/msstats_data_format.Rmd
Added Anthony Wu to package authors in MSstatsConvert.Rd. Reformatted .getFullDesign documentation argument entries (removed backticks, adjusted line wrapping). Rewrote the "Fractions and balanced design" vignette section to explicitly describe MSstatsBalancedDesign selection rules for overlapped features (label-free/SRM: highest non-missing MS run count; TMT: user-specified), row adjustments to ensure complete run/channel coverage, and NA handling for missing intensities. Added clarification distinguishing this fraction-collapsing logic from the summaryforMultipleRows parameter behavior in converters.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • Vitek-Lab/MSstatsConvert#127: The main PR's documentation expands/aligns the summaryforMultipleRows parameter semantics (e.g., max vs sum for TMT/label-free and duplicate PSM handling) for the same TMT converter functions introduced in the retrieved PR (#127, e.g., MaxQtoMSstatsTMTFormat, OpenMStoMSstatsTMTFormat, etc.).

Suggested reviewers

  • mstaniak

Poem

A rabbit hops through docs with care,
Clarifying parameters everywhere—
summaryforMultipleRows now shines clear,
Duplicate PSMs bring order near,
Roxygen2 config, fresh and bright! 🐰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning Pull request description is completely empty; no motivation, context, detailed changes, or testing information was provided by the author. Add a comprehensive PR description including: motivation/context for the documentation changes, detailed bullet-point list of all updates across files (DESCRIPTION, roxygen docs, man pages, vignette), description of any testing performed, and confirmation of the contributor checklist.
Title check ❓ Inconclusive The title 'Fix docs' is vague and generic; it does not clearly summarize the specific documentation changes (parameter clarifications, roxygen version updates, author metadata additions, vignette rewrites). Provide a more descriptive title that specifies the main documentation updates, such as 'Clarify summaryforMultipleRows parameter documentation across converters' or 'Update documentation for duplicate PSM handling and roxygen configuration'.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-docs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tonywu1999 tonywu1999 merged commit d7d0fb5 into devel May 27, 2026
2 checks passed
@tonywu1999 tonywu1999 deleted the fix-docs branch May 27, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant