Skip to content

Add option to use MS1Quantity for quantifications for Spectronaut#117

Merged
tonywu1999 merged 6 commits intodevelfrom
refactor-spec
Feb 10, 2026
Merged

Add option to use MS1Quantity for quantifications for Spectronaut#117
tonywu1999 merged 6 commits intodevelfrom
refactor-spec

Conversation

@tonywu1999
Copy link
Contributor

@tonywu1999 tonywu1999 commented Feb 9, 2026

Motivation and Context

Add option to use MS1Quantity for quantifications for Spectronaut. Right now, this is not possible.

Changes

  • Enable option to use MS1 quantification from Spectronaut.

Testing

Added unit tests for clean function.

Checklist Before Requesting a Review

  • I have read the MSstats contributing guidelines
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • New Features

    • Runtime selection and mapping of intensity sources (PeakArea, NormalizedPeakArea, MS1Quantity) so users can choose MS1-level quantification when MS2 is unreliable.
  • Documentation

    • Clarified intensity parameter descriptions and anomaly-scoring options, detailing available intensity choices and anomalyModelFeatures requirements.
  • Tests

    • Added a unit test validating MS1Quantity intensity handling and error on invalid intensity.
  • Chores

    • Removed prior pre-validation of the intensity parameter during input checks.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

Replaces hard-coded F handling with an intensity_column_mapping resolved at runtime, exposes three intensity options (PeakArea, NormalizedPeakArea, MS1Quantity) in docs and function defaults, updates callers/renames to use resolved column, adds a unit test, and removes an earlier intensity type-check.

Changes

Cohort / File(s) Summary
Core cleaning & mapping
R/clean_Spectronaut.R
Added intensity_column_mapping; resolve selected intensity at runtime; use mapped column for selection and renames instead of paste0("F", intensity); preserves anomaly scoring control flow.
Converter interface
R/converters_SpectronauttoMSstatsFormat.R
Made intensity parameter accept vector of allowed values ('PeakArea', 'NormalizedPeakArea', 'MS1Quantity') and updated related docs/signature formatting.
Validation utils
R/utils_checks.R
Removed prior explicit intensity type-validation block (intensity is no longer pre-validated there).
Unit tests
inst/tinytest/test_clean_Spectronaut.R
Added test exercising MS1Quantity mapping: sets FG.MS1Quantity, runs .cleanRawSpectronaut(intensity='MS1Quantity'), asserts output intensities and error on invalid intensity.
Documentation
man/MSstatsClean.Rd, man/SpectronauttoMSstatsFormat.Rd, man/dot-cleanRawSpectronaut.Rd
Expanded intensity descriptions to document three options and clarified calculateAnomalyScores / anomalyModelFeatures behavior; no signature changes in docs beyond parameter text.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Feature anomaly model #111: Modifies the Spectronaut conversion pipeline (including .cleanRawSpectronaut, SpectronauttoMSstatsFormat, and utils_checks) and appears directly related to anomaly-scoring and intensity handling changes.

Poem

🐰 I mapped the peaks with nimble paws,

From MS2 whispers to MS1 laws,
Three names I learn, one path I choose,
Columns align — no more to lose,
Hoppity clean, the data wows ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add option to use MS1Quantity for quantifications for Spectronaut' directly and accurately summarizes the main change in the changeset.
Description check ✅ Passed The PR description includes all required template sections: Motivation and Context (clear problem statement), Changes (concise bullet point), Testing (mentions unit tests added), and completed Checklist items.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-spec

No actionable comments were generated in the recent review. 🎉


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@R/clean_Spectronaut.R`:
- Around line 21-26: The code does not validate the intensity argument before
mapping, so intensity_column_mapping[[intensity]] can return NULL and silently
drop the intensity column; update the validation in the function that defines
intensity_column_mapping by ensuring intensity is one of the allowed keys (e.g.
use match.arg(intensity, choices = names(intensity_column_mapping)) or if (!
intensity %in% names(intensity_column_mapping)) stop(...)), then perform the
lookup for intensity_column; additionally, after computing intensity_column
check for NULL and raise a clear error mentioning the accepted options
(referencing intensity_column_mapping and intensity_column).
🧹 Nitpick comments (1)
inst/tinytest/test_clean_Spectronaut.R (1)

1-11: Good coverage of the happy path for MS1Quantity.

The test correctly validates that the MS1Quantity column value flows through to Intensity in the output.

Two suggestions:

  1. Consider adding a negative test for an invalid intensity value to ensure a clear error is raised (especially relevant if the validation fix from clean_Spectronaut.R is applied).
  2. Consider adding a test that verifies PeakArea still works as expected (regression guard).

,

@tonywu1999 tonywu1999 merged commit 35fd8af into devel Feb 10, 2026
2 checks passed
@tonywu1999 tonywu1999 deleted the refactor-spec branch February 10, 2026 19:08
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