Skip to content

Added nf-core module presto/pairseq - #12222

Open
Vivian0105 wants to merge 14 commits into
nf-core:masterfrom
Vivian0105:new_presto_pairseq
Open

Added nf-core module presto/pairseq#12222
Vivian0105 wants to merge 14 commits into
nf-core:masterfrom
Vivian0105:new_presto_pairseq

Conversation

@Vivian0105

Copy link
Copy Markdown
Contributor

Open issue: #12206

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.
  • Broadcast software version numbers to topic: versions - See version_topics
  • 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

Comment thread modules/nf-core/presto/pairseq/main.nf Outdated
Comment thread modules/nf-core/presto/pairseq/tests/main.nf.test
@SPPearce SPPearce added awaiting-changes will be closed after 30 days and removed Ready for Review labels Jul 15, 2026

@ggabernet ggabernet left a comment

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.

Agreed with @mashehu comments, otherwise it looks good to me

Comment thread modules/nf-core/presto/pairseq/tests/main.nf.test Outdated
Comment thread modules/nf-core/presto/pairseq/tests/main.nf.test
Comment thread modules/nf-core/presto/pairseq/meta.yml Outdated
@Vivian0105
Vivian0105 requested a review from mashehu July 29, 2026 21:03
@mashehu
mashehu force-pushed the new_presto_pairseq branch from 8e5a835 to 1b875e2 Compare July 29, 2026 21:47
@SPPearce SPPearce removed the awaiting-changes will be closed after 30 days label Jul 31, 2026
Comment thread modules/nf-core/presto/pairseq/tests/main.nf.test Outdated
Comment thread modules/nf-core/presto/pairseq/main.nf Outdated
Comment thread modules/nf-core/presto/pairseq/main.nf Outdated
Comment thread modules/nf-core/presto/pairseq/main.nf Outdated
Comment thread modules/nf-core/presto/pairseq/main.nf Outdated
output:
tuple val(meta), path("*1_pair-pass.fastq.gz"), path("*2_pair-pass.fastq.gz") , emit: reads
path "*_command_log.txt", emit: logs
tuple val("${task.process}"), val('presto'), eval('PairSeq.py --version | grep -o "[0-9][0-9.]*" | head -n 1'), emit: versions_presto, topic: versions

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.

Can you simplify this to one command (sed -n perhaps)? LLMs are very good at this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If use "sed", it will be:
tuple
val("${task.process}"),
val('presto'),
eval('PairSeq.py --version | sed -n "s/^PairSeq.py: \([0-9.]\)./\1/p"'),
emit: versions_presto,
topic: versions

For me, I feel it's harder to understand than grep + head.
Which one do you prefer?

Here's how PairSeq.py --version looks like (for your reference):
PairSeq.py: 0.7.9 2026.04.14

@Vivian0105
Vivian0105 requested a review from SPPearce July 31, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants