Skip to content

Conversation

@dhtt
Copy link

@dhtt dhtt commented Nov 5, 2025

PR checklist

Closes #XXX

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • If necessary, include test data in your PR.
  • Remove all TODO statements.
  • Emit the versions.yml file.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • 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:
      • nf-core modules test <MODULE> --profile docker
      • nf-core modules test <MODULE> --profile singularity
      • nf-core modules test <MODULE> --profile conda
    • For subworkflows:
      • nf-core subworkflows test <SUBWORKFLOW> --profile docker
      • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
      • nf-core subworkflows test <SUBWORKFLOW> --profile conda

@dhtt
Copy link
Author

dhtt commented Nov 5, 2025

Hi @famosab,

This PR is originates from PR #9290, which was done using a company git account. The new PR #9340 is now from a personal account. All previous comments can be viewed in this URL.

In this PR, I have added the following changes:

  • Add stub tests for vcf, vcf.gz, bcf, bcf.gz formats in main.nf.test
  • Update meta.yml file (keywords, output)
  • Reformat, linting

Please let me know if there are more changes to be made for this PR to be merge-able :) !
Many thanks!

Cheers,
Trang

@dhtt dhtt requested a review from famosab November 5, 2025 11:17
@dhtt dhtt moved this from To do to Ready for review in Hackathon Barcelona October 2025 Nov 5, 2025
@dhtt dhtt marked this pull request as ready for review November 5, 2025 11:19
@dhtt dhtt requested review from a team as code owners November 5, 2025 11:19
Copy link
Member

Choose a reason for hiding this comment

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

Hi @dhtt, we bump versions on separate PRs with renovate, so this file shouldn't be modified here

Copy link
Contributor

Choose a reason for hiding this comment

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

yes please remove this :)

assertAll(
{ assert process.success },
{ assert snapshot(
process.out
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
process.out
process.out,
path(process.out.versions[0]).yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you run nextflow lint on this :)

vembrane filter \\
${args} \\
${expression} \\
-o ${prefix}.filtered.vcf \\
Copy link
Contributor

Choose a reason for hiding this comment

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

In nf-core we prefer to let the user of the module handle the output name. So we rather make them check that the prefix is modified,.

Suggested change
-o ${prefix}.filtered.vcf \\
-o ${prefix}.vcf \\

also .vcf might not be correct maybe you can check the other vembrane module to see how this can be handled (as the output can be different formats).

script:
def args = task.ext.args ?: ''
def prefix = task.ext.prefix ?: "${meta.id}"

Copy link
Contributor

Choose a reason for hiding this comment

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

if ("${rds}" == "${prefix}.rds") error "Input and output names are the same, use \"task.ext.prefix\" to disambiguate!"

something in that direction :)

Copy link
Contributor

@famosab famosab left a comment

Choose a reason for hiding this comment

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

Looks good already!

@@ -0,0 +1,63 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/nf-core/modules/master/modules/meta-schema.json
name: "vembrane_filter"
description: write your description here
Copy link
Contributor

Choose a reason for hiding this comment

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

missing!

Copy link
Contributor

Choose a reason for hiding this comment

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

yes please remove this :)

@github-project-automation github-project-automation bot moved this from Ready for review to In progress in Hackathon Barcelona October 2025 Nov 6, 2025
Copy link
Contributor

@famosab famosab left a comment

Choose a reason for hiding this comment

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

Looks good otherwise - very nice work 🚀

val expression

output:
tuple val(meta), path("*.{vcf,bcf,bcf.gz}"), emit: filtered_variant
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I would just call the output channel vcf as well (is more in line with the other modules)

Suggested change
tuple val(meta), path("*.{vcf,bcf,bcf.gz}"), emit: filtered_variant
tuple val(meta), path("*.{vcf,bcf,bcf.gz}"), emit: vcf

ontologies: []

output:
filtered_variant:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
filtered_variant:
vcf:

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

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

5 participants