Major refactoring in the Isobaric workflow enabling to work similar to proteomicsLFQ#692
Draft
Major refactoring in the Isobaric workflow enabling to work similar to proteomicsLFQ#692
Conversation
Co-authored-by: Julianus Pfeuffer <8102638+jpfeuffer@users.noreply.github.com>
Co-authored-by: Julianus Pfeuffer <8102638+jpfeuffer@users.noreply.github.com>
… into isobaric_workflow
add IsobaricWorkflow to TMT workflow
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new ISOBARIC_WORKFLOW process is introduced to handle isobaric-labeled mass spectrometry analysis, replacing the prior feature-mapping and inference pipeline. The TMT workflow is refactored to integrate this module with a MSSTATS_CONVERTER component, and supporting configuration and metadata files are added. Changes
Sequence Diagram(s)sequenceDiagram
participant ch_file_prep as File Preparation
participant ch_id as ID Module
participant ch_join as Data Join
participant isobaric as ISOBARIC_WORKFLOW
participant converter as MSSTATS_CONVERTER
participant msstats as MSstats
ch_file_prep->>ch_join: mzmls (prepared spectra)
ch_id->>ch_join: id_results (identifications)
ch_join->>isobaric: joined mzmls & id_files
ch_join->>isobaric: expdesign (experimental design)
isobaric->>isobaric: sort by normalized base name
isobaric->>isobaric: run IsobaricWorkflow command
isobaric->>converter: out_consensusXML (quantification)
converter->>converter: convert consensus XML
converter->>msstats: out_msstats (formatted input)
msstats->>msstats: statistical analysis (optional)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Review Summary by QodoRefactor TMT workflow to use unified IsobaricWorkflow module
WalkthroughsDescription• Refactored TMT workflow to use IsobaricWorkflow instead of separate modules • Replaced FEATURE_MAPPER, FILE_MERGE, PROTEIN_INFERENCE, PROTEIN_QUANT with unified IsobaricWorkflow • Added MSSTATS_CONVERTER module for consensus XML to mzTab conversion • Simplified workflow pipeline architecture for isobaric quantification Diagramflowchart LR
ID["ID Subworkflow"]
ISO["ISOBARIC_WORKFLOW"]
CONV["MSSTATS_CONVERTER"]
MSSTATS["MSSTATS_TMT"]
ID -- "id_results" --> ISO
ISO -- "consensusXML" --> CONV
CONV -- "msstats_csv" --> MSSTATS
ISO -- "mzTab" --> RESULT["Final Results"]
File Changes1. conf/modules/modules.config
|
Contributor
Code Review by Qodo
1. Wrong isobaric type default
|
Up to standards ✅🟢 Issues
|
…rection - Use labelling_type from SDRF meta (auto-detected) instead of hardcoded params.type='itraq4plex' default - Pass isotope correction matrix when params.isotope_correction is enabled, matching the IsobaricAnalyzer module behavior - Pass extraction parameters (min_precursor_purity, precursor_isotope_deviation, min_reporter_intensity) that were missing from the IsobaricWorkflow invocation - Remove unused 'type' parameter from nextflow.config Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix: IsobaricWorkflow - auto-detect labelling type, pass isotope correction and extraction params
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/local/openms/isobaric_workflow/main.nf`:
- Around line 52-53: The command only forwards -picked_fdr and
-picked_decoy_string but omits the decoy-position option, so non-default
suffix/prefix decoy setups are ignored; update the invocation that sets
-picked_fdr ${params.picked_fdr} and -picked_decoy_string ${params.decoy_string}
to also pass the decoy position parameter (e.g., add the corresponding flag with
${params.decoy_string_position}) so the pipeline honors
params.decoy_string_position when running the picked-FDR step.
- Around line 23-39: The code rebuilds pairings by independently sorting mzmls
and id_files into mzml_sorted and id_sorted using extractBaseName, which can
silently mispair files if normalization misses a name; after computing
mzml_sorted and id_sorted, add a fast-fail check: ensure sizes match and then
iterate index-wise comparing extractBaseName(mzml_sorted[i].name) to
extractBaseName(id_sorted[i].name), and if any mismatch occurs throw an
exception (or call error/exit) with a clear message listing the offending
pair(s) and their original names so the pipeline fails immediately rather than
proceeding with wrong ID/mzML pairing.
In `@nextflow.config`:
- Around line 181-182: The repo adds a separate params.key "type" (value
'itraq4plex') which is unused by existing runs that set params.labelling_type,
because the workflow reads params.type; fix by either removing the duplicated
"type" setting from the config or making it derive from the existing
labelling_type (e.g., set type = params.labelling_type ?: 'itraq4plex'), or
change the workflow to read params.labelling_type instead of params.type; update
references to use the single canonical symbol (labelling_type) so existing CLI
flags continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2aafdde8-9873-46f2-8b2e-d277e8e2568d
📒 Files selected for processing (5)
conf/modules/modules.configmodules/local/openms/isobaric_workflow/main.nfmodules/local/openms/isobaric_workflow/meta.ymlnextflow.configworkflows/tmt.nf
Comment on lines
+23
to
+39
| def extractBaseName = { filename -> | ||
| def name = filename.toString() | ||
| name = name.replaceAll(/\.mzML$/, '') | ||
|
|
||
| if (name.endsWith('.idXML')) { | ||
| name = name.replaceAll(/\.idXML$/, '') | ||
| name = name.replaceAll(/_(comet|msgf|sage|consensus)(_perc)?(_filter)?(_fdr)?$/, '') | ||
| } | ||
| return name | ||
| } | ||
|
|
||
| def mzml_sorted = mzmls.collect().sort{ a, b -> | ||
| extractBaseName(a.name) <=> extractBaseName(b.name) | ||
| } | ||
| def id_sorted = id_files.collect().sort{ a, b -> | ||
| extractBaseName(a.name) <=> extractBaseName(b.name) | ||
| } |
Contributor
There was a problem hiding this comment.
Fail fast if the mzML/idXML pairing drifts.
Upstream these files are already matched by sample, but this process rebuilds that pairing by sorting both lists independently and then passing them positionally to -in / -in_id. If a filename falls outside the current normalization regex, the command still runs but can quantitate against the wrong ID file.
🛠️ Suggested guard
def mzml_sorted = mzmls.collect().sort{ a, b ->
extractBaseName(a.name) <=> extractBaseName(b.name)
}
def id_sorted = id_files.collect().sort{ a, b ->
extractBaseName(a.name) <=> extractBaseName(b.name)
}
+ def mzml_names = mzml_sorted.collect { extractBaseName(it.name) }
+ def id_names = id_sorted.collect { extractBaseName(it.name) }
+ if (mzml_names != id_names) {
+ throw new IllegalArgumentException("Mismatched IsobaricWorkflow inputs: mzML=${mzml_names} idXML=${id_names}")
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/local/openms/isobaric_workflow/main.nf` around lines 23 - 39, The
code rebuilds pairings by independently sorting mzmls and id_files into
mzml_sorted and id_sorted using extractBaseName, which can silently mispair
files if normalization misses a name; after computing mzml_sorted and id_sorted,
add a fast-fail check: ensure sizes match and then iterate index-wise comparing
extractBaseName(mzml_sorted[i].name) to extractBaseName(id_sorted[i].name), and
if any mismatch occurs throw an exception (or call error/exit) with a clear
message listing the offending pair(s) and their original names so the pipeline
fails immediately rather than proceeding with wrong ID/mzML pairing.
Comment on lines
+52
to
+53
| -picked_fdr ${params.picked_fdr} \\ | ||
| -picked_decoy_string ${params.decoy_string} \\ |
Contributor
There was a problem hiding this comment.
Propagate the decoy-position setting in this new path.
The pipeline already exposes params.decoy_string_position, but this command only forwards the decoy string. Any non-default suffix-decoy setup is ignored here, so the isobaric workflow no longer honors the full picked-FDR decoy configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@modules/local/openms/isobaric_workflow/main.nf` around lines 52 - 53, The
command only forwards -picked_fdr and -picked_decoy_string but omits the
decoy-position option, so non-default suffix/prefix decoy setups are ignored;
update the invocation that sets -picked_fdr ${params.picked_fdr} and
-picked_decoy_string ${params.decoy_string} to also pass the decoy position
parameter (e.g., add the corresponding flag with
${params.decoy_string_position}) so the pipeline honors
params.decoy_string_position when running the picked-FDR step.
This PR is against the
|
|
Contributor
CI Feedback 🧐(Feedback updated until commit 69d5c61)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
Collaborator
|
Why do you merge against master? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).Summary by CodeRabbit