Feature visualization - #180
Open
KarinSchork wants to merge 22 commits into
Open
Conversation
|
julianu
force-pushed
the
feature-visualization
branch
from
August 20, 2026 12:47
c0367db to
811365d
Compare
|
❌ nf-test failed with latest Nextflow versionNote Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
See the full run for details. |
julianu
requested changes
Aug 20, 2026
| @@ -0,0 +1,10 @@ | |||
| name,sequence,mz,RT,mz-tol,rt-tol | |||
| MPCSPIKE1,X[+457.2834],457.2834,5,10 ppm,36000 | |||
Member
There was a problem hiding this comment.
Maybe as the default we could add something really useful, like iRT peptides (and have a test-dataset with them)?
The MPC spike in peptides can still be used, but only for the respective dataset, IMHO
Member
Author
There was a problem hiding this comment.
This would be more of a global thing right? So to use such a data set for all tests?
Member
There was a problem hiding this comment.
Yes, this would be a global default. But the peptides right now are rather unknown and very MPC-specific.
| saveAs: { filename -> filename.equals('versions.yml') ? null : filename } | ||
| ] | ||
| ext.spikein_columns = { params.spikein_columns ?: 'MS1 feature maximum intensity,retention time,count of identified spectra,Delta_to_expected_RT' } | ||
| ext.args = { |
Member
There was a problem hiding this comment.
Using all the arguments as ext.args is a nasty hack, which should be omitted.
Granted, I kind of did this in cometconfig, BUT there is no nice way to pass an argument to a (python) template. Calling a python script and passing the params in the script, as in your visualization module, allows to pass params from teh module to the script, though.
ext.args in this case should rather be used to "pass additional, not mandatory params to the tool" (in your case the python module). But your params seem to be mandatory. So please make these passed params to the module (like teh outputdir), instead of squeezing them into the ext.args.
Member
There was a problem hiding this comment.
Now, you can safely remove the withName: 'QCVISUALIZATION'.. in modules.config, as it uses the default. The outputdir is set by the process-name.
|
|
||
| conda "${moduleDir}/environment.yml" | ||
| container "${ workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container ? | ||
| 'https://depot.galaxyproject.org/singularity/YOUR-TOOL-HERE': |
Member
There was a problem hiding this comment.
I think you could put the ghcr container here onle - the apptainer-stub shoudl be removed (but not urgent now)
Member
Author
There was a problem hiding this comment.
I changed it to how it was done in cometconfig, I hope thats ok
Member
There was a problem hiding this comment.
Maybe you missed the push here? It is still there.
But more urgently: please use a running version of the helper Docker container. "latest" is not updated, so the executer uses the "latest it has cached", which can be an old version.
For me, ghcr.io/mpc-bioinformatics/macproqc-helpers:sha-60c25b7 works right now and produces non-empty files.
You also need to update the test-snapshot 😉
julianu
reviewed
Aug 25, 2026
julianu
left a comment
julianu
left a comment
Member
There was a problem hiding this comment.
Added a new comment regarding the stub test.
| ) | ||
| } | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
Please add a "-stub" test as well.
julianu
reviewed
Aug 25, 2026
julianu
left a comment
julianu
left a comment
Member
There was a problem hiding this comment.
Some more requests for the documentation.
| | `name` | Unique identifier for the spike-in peptide. | | ||
| | `sequence` | Proforma peptidoform sequence of the spike-in, including any modifications. | | ||
| | `mz` | Expected mass-to-charge ratio (m/z) of the spike-in. | | ||
| | `RT` | Expected retention time of the spike-in. | |
Member
There was a problem hiding this comment.
Please add the unit information.
| - [FDR filtering and protein inference](#fdr-filtering-and-protein-inference) — 1 % FDR filter and PIA protein groups _(migrating)_ | ||
| - [Feature finding](#feature-finding) — isotope feature detection with IDMapper _(migrating)_ | ||
| - [mzQC output](#mzqc-output) — standardised QC metric export _(migrating)_ | ||
| - [Visualisation](#visualisation) — interactive Plotly report _(migrating)_ |
Member
There was a problem hiding this comment.
Please update this information and the link
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
2 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.
Add subworkflow and module for visualization
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).