fix(turnover): Fix DIANN 1.8 processing of protein turnover labeling#140
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIsotope-label suffix parsing in DIANN ModifiedSequence handling now accepts any non-dash label token before the amino-acid H/L suffix. Regexes, warnings, tests, and documentation are updated, and internal ProteinProspector documentation is generated. ChangesGeneric isotope-label suffix parsing
ProteinProspector documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
R/clean_DIANN.R (1)
268-276: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the three regex constructions.
heavy_regex,light_regex, andstrip_regexrepeat the same\\([^-]+-(?:aa_pattern)-...scaffold with only the trailing suffix differing. A single pattern with a capture group (e.g. matching-([HL])\)) could drive both classification and stripping, reducing the chance of the three patterns drifting out of sync during future edits.♻️ Possible consolidation
- heavy_regex <- paste0("\\([^-]+-(?:", aa_pattern, ")-H\\)") - light_regex <- paste0("\\([^-]+-(?:", aa_pattern, ")-L\\)") - strip_regex <- paste0("\\([^-]+-(?:", aa_pattern, ")-[HL]\\)") + label_regex <- paste0("\\([^-]+-(?:", aa_pattern, ")-([HL])\\)") + heavy_regex <- paste0("\\([^-]+-(?:", aa_pattern, ")-H\\)") + light_regex <- paste0("\\([^-]+-(?:", aa_pattern, ")-L\\)") + strip_regex <- label_regex🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@R/clean_DIANN.R` around lines 268 - 276, The three regex variables in clean_DIANN reuse the same scaffold and should be consolidated to avoid drift. Update the regex construction near .classifyIsotopeLabelType so a single shared pattern (with a capture for the H/L suffix) can be used to derive heavy_regex, light_regex, and strip_regex consistently, while keeping the current behavior in dn_input and PeptideSequence stripping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@R/clean_DIANN.R`:
- Around line 268-276: The three regex variables in clean_DIANN reuse the same
scaffold and should be consolidated to avoid drift. Update the regex
construction near .classifyIsotopeLabelType so a single shared pattern (with a
capture for the H/L suffix) can be used to derive heavy_regex, light_regex, and
strip_regex consistently, while keeping the current behavior in dn_input and
PeptideSequence stripping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfd2c1d6-44a4-4b94-b81f-368001b9392e
📒 Files selected for processing (6)
R/clean_DIANN.RR/converters_DIANNtoMSstatsFormat.Rinst/tinytest/test_clean_DIANN.Rman/DIANNtoMSstatsFormat.Rdman/MSstatsClean.Rdman/dot-cleanRawDIANN.Rd
Motivation and context
DIANN 1.8 turnover labeling in MSstatsConvert was not handling
ModifiedSequencelabels flexibly enough. The fix generalizes isotope-label parsing so turnover/labeled peptides can be recognized even when the label token is not literallySILAC.Summary of solution
The DIANN parsing logic was updated to detect generic label suffixes of the form
(<label>-<AA>-H/L)and to strip them from peptide sequences while assigning the correct isotope label type.Detailed changes
.assignDIANNIsotopeLabelType()to support generic label tokens instead of onlySILAC.ModifiedSequenceparsing regexes to match:(<label>-<AA>-H)(<label>-<AA>-L)where
<label>is any non-dash token and<AA>is restricted to the configured labeled amino acids.SILAC-K-H/L(label-K-H/L)verifying correct
IsotopeLabelTypeassignment and peptide stripping.Tests
inst/tinytest/test_clean_DIANN.Rfor generalizedModifiedSequencelabel parsing.Coding guidelines
Motivation and Context
DIANN 1.8 can encode protein-turnover labels in
ModifiedSequenceusing label tokens beyondSILAC. Previously, MSstatsConvert recognized only SILAC-specific suffixes, preventing correct classification of generic labels.The parser now supports
(<label>-<AA>-H/L)suffixes with configured labeled amino acids, strips recognized annotations, and preserves existing channel-based DIANN behavior.Changes
SILAC-K-H/Llabel-K-H/L.cleanRawProteinProspector.Unit Tests
.assignDIANNIsotopeLabelTypetests to verify:H/Lclassification.NAfor unlabeled peptides.PeptideSequence.Coding Guidelines