Skip to content

Add varnet (https://github.com/skandlab/VarNet)#11787

Open
kiranchari wants to merge 16 commits into
nf-core:masterfrom
kiranchari:add-varnet
Open

Add varnet (https://github.com/skandlab/VarNet)#11787
kiranchari wants to merge 16 commits into
nf-core:masterfrom
kiranchari:add-varnet

Conversation

@kiranchari

Copy link
Copy Markdown

PR checklist

Closes #XXX

  • [add varnet somatic variant caller] This comment contains a description of changes (with reason).
  • [yes] If you've added a new tool - have you followed the module conventions in the contribution docs
  • [tests passed] If necessary, include test data in your PR.
  • [done] Remove all TODO statements.
  • [yes] Broadcast software version numbers to topic: versions - See version_topics
  • [yes] Follow the naming conventions.
  • [yes] Follow the parameters requirements.
  • [yes] Follow the input/output options guidelines.
  • Use BioConda and BioContainers if possible to fulfil software requirements.
  • Ensure that the test works with either Docker / Singularity. Conda CI tests can be quite flaky:
    • For modules:
      • [done] nf-core modules test <MODULE> --profile docker

Comment thread modules/nf-core/varnet/main.nf Outdated
Comment on lines +50 to +53
cat <<-END_VERSIONS > \$WORKDIR/versions.yml
"${task.process}":
varnet: 1.5.0
END_VERSIONS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please switch to topics.
cf https://nf-co.re/blog/2025/version_topics for example

Comment thread modules/nf-core/varnet/meta.yml Outdated
Comment on lines +82 to +88
vcf_tbi:
- - meta:
type: map
description: |
Groovy Map containing sample information
e.g. `[ id:'sample1', single_end:false ]`
- ${prefix}/${prefix}.vcf.gz.tbi: {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we're missing this in the main.nf

No tbi are being emitted out

@kiranchari

Copy link
Copy Markdown
Author

@maxulysse I have updated the pull request. Thank you

@maxulysse

Copy link
Copy Markdown
Member

@maxulysse I have updated the pull request. Thank you

The snapshots need to be updated too. They appear empty at the moment

@kiranchari

Copy link
Copy Markdown
Author

@maxulysse I have updated the pull request. Thank you

The snapshots need to be updated too. They appear empty at the moment

@maxulysse Pull requested updated. Thank you

Comment thread modules/nf-core/varnet/tests/main.nf.test Outdated
Comment thread modules/nf-core/varnet/tests/main.nf.test Outdated
Comment thread modules/nf-core/varnet/tests/nextflow.config
Comment thread modules/nf-core/varnet/main.nf Outdated
Comment thread modules/nf-core/varnet/main.nf
kiranchari and others added 4 commits June 3, 2026 20:11
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
TF_CPP_MIN_LOG_LEVEL=3 python /VarNet/filter.py \\
--sample_name ${prefix} \\
${normal} \\
--tumor_bam \$WORKDIR/${input_tumor} \\

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really need to use WORKDIR for everything? Given it is your tool, could you patch it to respect the current working directory?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I modified this because the test input files were using relative paths based on $WORKDIR instead of absolute paths, which was causing the tests to fail. Prepending $WORKDIR was a quick fix to make those paths valid so the tests could run.

tag "$meta.id"
label 'process_high_memory'

container "docker.io/kiranchari/varnet:latest"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a plan to put it on bioconda?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at the moment!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that a nf-core recommendation describes this: https://nf-co.re/docs/specifications/pipelines/recommendations/bioconda

We do make some exceptions for this but usually only for things like licensing issues etc. We have a #bioconda channel on slack with a bunch of experts around to help :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My institution requires VarNet to use a PolyForm Noncommercial License 1.0.0, which prevents adding it to bioconda. Please let me know if my understanding is incorrect.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never heard of that one before.
From what I can tell, it allows for redistribution, which I think would mean it is compatible with bioconda. But I'm not a bioconda expert.

Comment thread modules/nf-core/varnet/main.nf Outdated
Comment thread modules/nf-core/varnet/main.nf Outdated
@famosab

famosab commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Please join the nf-core organization on GitHub to enable the CI-tests to run on your PR. You can request to join the organization via #github-invitations in the nf-core slack. You can join the nf-core slack via https://nf-co.re/join.

@kiranchari

Copy link
Copy Markdown
Author

Please join the nf-core organization on GitHub to enable the CI-tests to run on your PR. You can request to join the organization via #github-invitations in the nf-core slack. You can join the nf-core slack via https://nf-co.re/join.

Done!

Comment thread modules/nf-core/varnet/tests/main.nf.test Outdated

WORKDIR=\$(pwd)

cd /VarNet

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you changing the directory to here? Hence you obviously need the WORKDIR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, my scripts rely on being in the /VarNet working directory. The test input files were using relative paths based on $WORKDIR instead of absolute paths, which was causing the tests to fail. Prepending $WORKDIR was a quick fix to make those paths valid so the tests could run.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest with you, it sounds like you need to update your tool to make it functional.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To elaborate a little: a key feature of Nextflow is that execution of each task is encapsulated within its work directory. That way, different tasks do not affect one another and the execution becomes much safer and more reproducible.

Moving outside of the work directory violates this, and also means the pipeline is much less likely to work in diverse compute environments. eg. You typically cannot make root directories like this if you're running on a HPC using conda.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants