Skip to content

Conversation

@maxibor
Copy link
Member

@maxibor maxibor commented Oct 17, 2025

PR checklist

RiboDetector is a software developed to accurately yet rapidly detect and remove rRNA sequences from metagenomeic, metatranscriptomic, and ncRNA sequencing data.

  • This comment contains a description of changes (with reason).
  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • 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 singularity

input:
tuple val(meta), path(fastq)
val length

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

Comment on lines 16 to 18
tuple val(meta), path("*.nonrna*.fastq.gz"), emit: fastq
tuple val(meta), path("*.log"), emit: log
path "versions.yml" , emit: versions
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
tuple val(meta), path("*.nonrna*.fastq.gz"), emit: fastq
tuple val(meta), path("*.log"), emit: log
path "versions.yml" , emit: versions
tuple val(meta), path("*.nonrna*.fastq.gz"), emit: fastq
tuple val(meta), path("*.log") , emit: log
path "versions.yml" , emit: versions

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.

Suggested change

END_VERSIONS
"""


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

Comment on lines 55 to 56
touch ${prefix}.nonrna.1.fastq.gz
touch ${prefix}.nonrna.2.fastq.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

nf-test doesn't like empty files with .gz as the suffix, it can give weird errors.

Suggested change
touch ${prefix}.nonrna.1.fastq.gz
touch ${prefix}.nonrna.2.fastq.gz
echo | gzip > ${prefix}.nonrna.1.fastq.gz
echo | gzip > ${prefix}.nonrna.2.fastq.gz

Copy link
Member Author

Choose a reason for hiding this comment

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

Good to know, thanks !

- genomics
tools:
- "ribodetector":
description: "Accurate and rapid RiboRNA sequences Detector based on deep learning"
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
description: "Accurate and rapid RiboRNA sequences Detector based on deep learning"
description: "Accurate and rapid RiboRNA sequences detector based on deep learning"

Comment on lines 28 to 33
assertAll(
{ assert process.success },
{ assert process.out.fastq },
{ assert process.out.log },
{ assert path(process.out.log[0][1]).getText().contains("Writing output non-rRNA sequences") },
)
Copy link
Contributor

Choose a reason for hiding this comment

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

The assertions need at least to check that the versions.yml is produced consistently.
Are the files stable too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, the output can't consistently checksumed between two runs :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not, what is changing in the fastqs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a random seed that needs to be set?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, AFAIK, the devs of Ribodetector haven't implemented a way to set the manual seed in Pytorch, so 2 different runs will give slightly different results.

@maxibor maxibor requested a review from SPPearce October 24, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants