Handling DIANN parameters to get extended parameters to modules validated #660
Handling DIANN parameters to get extended parameters to modules validated #660ypriverol merged 25 commits intobigbio:devfrom
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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.
Pull request overview
This pull request enhances the DIA-NN workflow by adding support for custom command-line arguments via a new diann_extra_args parameter. The changes ensure that a single DIA-NN configuration file is shared across all DIA-NN processing steps, and introduce validation logic to prevent incompatible flags from being passed to specific DIA-NN steps.
Changes:
- Added
diann_extra_argsparameter to allow users to pass custom DIA-NN arguments - Modified DIA workflow to convert the configuration channel to a value channel for reuse across multiple processes
- Updated all DIA-NN modules (except INSILICO_LIBRARY_GENERATION) to receive and use the diann_config file for extracting modification flags
- Implemented validation function to strip incompatible flags with warnings for each DIA-NN step
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/dia.nf | Converts diann_cfg to value channel and passes it to all DIA-NN modules that need it |
| modules/local/diann/preliminary_analysis/main.nf | Added diann_config input parameter and modification flag extraction logic |
| modules/local/diann/individual_analysis/main.nf | Added diann_config input parameter and modification flag extraction logic |
| modules/local/diann/final_quantification/main.nf | Added diann_config input parameter and modification flag extraction logic |
| modules/local/diann/assemble_empirical_library/main.nf | Added diann_config input parameter and modification flag extraction logic |
| conf/modules/modules.config | Added validateDiannExtraArgs function and updated ext.args for all DIA-NN steps to use it |
| nextflow.config | Added diann_extra_args parameter with default null value |
| nextflow_schema.json | Added schema definition for diann_extra_args parameter |
| ro-crate-metadata.json | Added clarification about supported SDRF file extensions (.sdrf, .tsv, .csv) |
| .gitignore | Added .claude/ directory to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
In theory not bad. Did you ask at nf-core how other people solved that so far? |
Im now in the nf-core channel discussing that. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| blocked.sort { a -> -a.length() }.each { flag -> | ||
| def flagPattern = '(?<=^|\\s)' + java.util.regex.Pattern.quote(flag) + '(?=\\s|\$)(\\s+(?!-{1,2}[a-zA-Z])\\S+)*' | ||
| if (args =~ flagPattern) { | ||
| log.warn "DIA-NN: '${flag}' is managed by the pipeline for INSILICO_LIBRARY_GENERATION and will be stripped." |
There was a problem hiding this comment.
Maybe add some hint on where the actual parameters that manage that can be found.
There was a problem hiding this comment.
I still think this should be addressed. "Managed by the pipeline" wont help users if they are trying to change a parameter and get that warning.
https://nfcore.slack.com/archives/CE6SDBX2A/p1771582947571129 |
|
Is DIANN updated? |
|
This PR is related also with the new release of quantms refactoring going on: #663 |
Lukas-Neuhauser-HMS
left a comment
There was a problem hiding this comment.
Hi @ypriverol,
I went through the PR. It neatly serves our needs. I have no further comments.
Also, cool that you addressed that diann_normalize was not passed to DIANN before and that performance_mode does now not block ext.args of PRELIMINARY_ANALYSIS anymore.
Thank you!
Lukas
|
Thanks @Lukas-Neuhauser-HMS . Also for triggering more discussions in quantms #663 Please let us know if we have some more questions in those issues. |
| // MODULE: INDIVIDUAL_ANALYSIS | ||
| // | ||
| INDIVIDUAL_ANALYSIS(indiv_fin_analysis_in) | ||
| INDIVIDUAL_ANALYSIS(indiv_fin_analysis_in, ch_diann_cfg) |
There was a problem hiding this comment.
Do all DIA modules require the ch_diann_cfg parameter now, eg modifications?
There was a problem hiding this comment.
Nice catch; let me double check this.
|
I was reading a bit on |
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).