Repository navigation
New module: onkopus/run - #13387
New module: onkopus/run#13387vagkaratzas wants to merge 7 commits into
Conversation
Annotate, interpret and prioritise cancer variants in a VCF with Onkopus (https://gitlab.gwdg.de/MedBioinf/mtb/onkopus/onkopus). Onkopus is only on PyPI, so the environment installs it with pip and uses Seqera Containers. The tool queries the public Onkopus web services, so the real test needs internet access. Closes #9211 Generated by nf-core-module-dev v1.7.0 (Claude Opus 5.5, high effort, as manager; Claude Sonnet 5.5, medium effort, as subagents) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
Did you open an issue? Can you link it here? |
famosab
left a comment
There was a problem hiding this comment.
Thanks for picking this up :) just a few small comments
| -g "${genome}" \\ | ||
| ${args} | ||
|
|
||
| gzip -c "${prefix}.tmp.vcf" > "${prefix}.ann.vcf.gz" |
There was a problem hiding this comment.
We need vcf files to be bgzipped not only gzipped even though the extension is always only called .gz instead of .bgz.
There was a problem hiding this comment.
Done in fd3b5b4: added bioconda::htslib=1.24 to the environment (containers rebuilt) and the script and stub now write the output with bgzip.
Comment added by Claude Opus 5.5
| description: Variant calls in VCF format, optionally gzip-compressed | ||
| pattern: "*.{vcf,vcf.gz}" | ||
| ontologies: | ||
| - edam: http://edamontology.org/format_3016 # VCF | ||
| - edam: http://edamontology.org/format_3989 # GZIP format |
There was a problem hiding this comment.
I think we need bgzipped files here aswell.
There was a problem hiding this comment.
Done in fd3b5b4: input now uses the bgzip EDAM term (format_3615); plain VCF is still accepted.
Comment added by Claude Opus 5.5
| description: Annotated variants in gzip-compressed VCF format | ||
| pattern: "*.ann.vcf.gz" | ||
| ontologies: | ||
| - edam: http://edamontology.org/format_3016 # VCF | ||
| - edam: http://edamontology.org/format_3989 # GZIP format |
There was a problem hiding this comment.
I think we need bgzipped files here aswell.
There was a problem hiding this comment.
Done in fd3b5b4: output now uses the bgzip EDAM term (format_3615).
Comment added by Claude Opus 5.5
| input[0] = channel.of( | ||
| "##fileformat=VCFv4.2", | ||
| "#CHROM\\tPOS\\tID\\tREF\\tALT\\tQUAL\\tFILTER\\tINFO", | ||
| "chr7\\t140753336\\t.\\tA\\tT\\t.\\t.\\t." | ||
| ).collectFile(name: 'braf_v600e.vcf', newLine: true).map { f -> tuple([ id:'test' ], f) } | ||
| input[1] = 'hg38' |
There was a problem hiding this comment.
Nice way to test but I would like to see a full vcf being tested (can be with only a few variants but this is very simplified).
There was a problem hiding this comment.
Done in fd3b5b4: the test now uses genomics/homo_sapiens/illumina/pharmcat/test.chr22.single.sample.vcf.gz from test-datasets (156 chr22 variants, 29 get ClinVar annotations; about 11 s against the Onkopus service). Smaller test VCFs had no ClinVar hits, so the snapshot would have shown no annotation.
Comment added by Claude Opus 5.5
Address review on #13387: - compress the annotated VCF with bgzip (htslib added; containers rebuilt) - mark input/output VCF as bgzip (EDAM format_3615) in meta.yml - test on a real hg38 VCF from test-datasets and snapshot its md5 - fix environment.yml indentation flagged by prettier Generated by Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Not yet: Onkopus is hosted on the GWDG GitLab (https://gitlab.gwdg.de/MedBioinf/mtb/onkopus/onkopus). The full list of limitations is in the PR description, ready to forward to the authors if you have a contact there. Comment added by Claude Opus 5.5 |
| ).collectFile(name: 'braf_v600e.vcf', newLine: true).map { f -> tuple([ id:'test' ], f) } | ||
| input[0] = channel.of([ | ||
| [ id:'test' ], | ||
| file(params.modules_testdata_base_path + 'genomics/homo_sapiens/illumina/pharmcat/test.chr22.single.sample.vcf.gz', checkIfExists: true) |
There was a problem hiding this comment.
why the pharmcat vcf? and a more generic one also used with VEP
There was a problem hiding this comment.
Also I am surprised that the md5sum seems to be stable usually that is not the case with bgzipped vcfs and you have to assert them specifically so I am not sure if it works as expected
There was a problem hiding this comment.
VEP's test VCF is SARS-CoV-2 (genomics/sarscov2/illumina/vcf/test.vcf), and Onkopus annotates human hg38 only, so it would come back with no annotation. The generic human test VCFs I tried had no ClinVar hits either, which makes for an empty snapshot. The pharmcat file is just a human hg38 chr22 VCF that happens to live in the pharmcat folder; 29 of its 156 variants get ClinVar annotations. Happy to switch if you know a more generic human VCF with ClinVar-relevant variants.
Comment added by Claude Opus 5.5
| - conda-forge::python=3.12.15 | ||
| - pip: | ||
| - onkopus==0.7.0 | ||
| - onkopus==0.7.0 |
There was a problem hiding this comment.
Did you open an issue over at their github to supply the tool as conda package? :)
There was a problem hiding this comment.
They have an institutional GitLab, so can't post an Issue, and can't Register.
https://gitlab.gwdg.de/MedBioinf/mtb/onkopus/onkopus
There was a problem hiding this comment.
ok let me send the author an email (she works at my institute now haha)
There was a problem hiding this comment.
I asked whether it would be ok for them if we just create the conda package, thats really easy when a pip package is available. I can take care of that. Lets wait for her response :)
| echo "${args}" | ||
|
|
||
| echo "" | gzip > "${prefix}.ann.vcf.gz" | ||
| echo "" | bgzip > "${prefix}.ann.vcf.gz" |
There was a problem hiding this comment.
For stub its not really important in my eyes but we can do it like this :)
I know someone who worked on onkopus but I think they would still prefer to have an issue opened. |
vagkaratzas
left a comment
There was a problem hiding this comment.
- Versions:
htslibandpythonneed their ownversions_*emits. - Real test: snapshot
sanitizeOutput(process.out)instead of hand-picking channels. - The PR description is out of date with the code: it mentions
gzip(code usesbgzipfrom htslib), a one-variant BRAF V600E VCF built inside the test (test uses the pharmcat chr22 VCF), a ClinVar-field check instead of a checksum (test snapshotsvariantsMD5), and the oldpython_pip_onkopuscontainer. Please refresh it.
Diff 8.81/10 · Overall 8.81/10
nf-core-reviewer 0.1.0 / vagkaratzas-review (rubric v1) · claude-opus-5-5
Generated by Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds
onkopus/run, which annotates, interprets and prioritises cancer variants in a VCF with Onkopus (Kurz et al. 2025, NAR).@famosab, since you opened #9211, could you review this? Thanks!
Notes for review
onkopus/runbecause the tool has several subcommands (run,install,start,list-modules, …).environment.ymltherefore uses conda-forge python and pip withonkopus==0.7.0, and the containers are Seqera Containers built from it.onkopus runsends variants to the public Onkopus web services. Tasks need internet access, and patient data leave the compute environment. Both points are documented inmeta.yml. The real test therefore depends on that service: it annotates one variant (BRAF V600E) with-m clinvarand checks that a ClinVar field is added, instead of snapshotting a checksum.PYTHONPATH.--versionflag, so the version is read withimportlib.metadata.tuple val(meta), path(vcf)andval genome(hg19,hg38ort2t). The onkopus annotation modules (-m) and the liftover target genome (-t) go throughext.args.Upstream limitations (to be reported to the Onkopus authors)
Onkopus v0.7.0: 7 issues found while packaging
Container/env:
community.wave.seqera.io/library/python_pip_onkopus:cbfd43c4529368f9(Seqera Containers), PyPIonkopus==0.7.0(pullsadagenes==0.5.3)Found while packaging the tool as an nf-core module.
1. Package writes into its own install directory at import
adagenes/onkopuscreates directories next to the package source (adagenes/conf/uploaded_files,site-packages/data), which fails with "Permission denied" in a read-only image, even with--writable-tmpfs.onkopus --helpfails for the same reason../pyliband prepends it toPYTHONPATH(modules/nf-core/onkopus/run/main.nf:26-30).$TMPDIRor anONKOPUS_DATA_DIRenv variable), never inside the installed package.2. No
--versionflagonkopus --version).python -c "import importlib.metadata as m; print(m.version('onkopus'))"(main.nf:17).onkopus --version.3. Default mode sends variant data to a public web service
onkopus runsends variants tohttps://mtb.bioinf.med.uni-goettingen.de; offline use needs Docker-run module servers (onkopus install/start), which cannot run inside a module container. Tests depend on the live service and its current annotation content.meta.yml; tests assert structure (a ClinVar field present) instead of file checksums (tests/main.nf.test).4. Compressed output not documented
.vcf.gz) directly.-oextension; gzip output support is undocumented.${prefix}.tmp.vcf, thengzipit (main.nf:32-38).-o out.vcf.gz(bgzip).5. Several
-mmodules may overwrite each other (from source, not tested)-m a,bapplies all selected modules to one output.meta.ymladvises one-mmodule per process; tests use-m clinvaronly.6. Liftover writes a temporary file with a trailing space in its name (from source, not tested)
<input>.GRCh38.vcffile (trailing space) is written next to the input.7. Noisy
SyntaxWarningon stderradagenesemits PythonSyntaxWarningmessages (invalid escape sequences) on every run.adagenes.Generated by nf-core-module-dev v1.7.0 (Claude Opus 5.5, high effort, as manager; Claude Sonnet 5.5, medium effort, as subagents)
PR checklist
Closes #9211
topic: versions- See version_topicslabelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile conda🤖 Generated with Claude Code