Skip to content

Conversation

@Ales-ibt
Copy link

nf-core/modules pull request

This is a brand new module for rpsblast, a BLAST application that searches a protein query against the conserved domain database (CDD).

PR checklist

Reported in issue #

  • If you've added a new tool - have you followed the module conventions in the contribution docs
  • Remove all TODO statements.
  • Follow the naming conventions.
  • Follow the parameters requirements.
  • Follow the input/output options guidelines.
  • Add a resource label
  • 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

@Ales-ibt Ales-ibt requested a review from vagkaratzas December 16, 2025 15:32
@Ales-ibt Ales-ibt self-assigned this Dec 16, 2025
Copy link
Contributor

@vagkaratzas vagkaratzas left a comment

Choose a reason for hiding this comment

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

Nice, left some comments!

documentation: https://blast.ncbi.nlm.nih.gov/Blast.cgi?CMD=Web&PAGE_TYPE=Blastdocs
doi: 10.1016/S0022-2836(05)80360-2
licence: ["US-Government-Work"]
identifier: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you can add the blast identifier here, since rpsblast is shipped with the main blast container, correct?

Copy link
Author

Choose a reason for hiding this comment

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

I got the message from nf-core that the blast identifier couldn't be found during the module creation. I have checked the other nf-core modules for blast and any of them have identifier. Where else can I check?

Copy link
Contributor

@vagkaratzas vagkaratzas left a comment

Choose a reason for hiding this comment

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

Some missed things

script "../../../wget/main.nf"
process {
"""
input[0] = [ [id:'smart'], "https://ftp.ncbi.nih.gov/pub/mmdb/cdd/little_endian/Smart_LE.tar.gz" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment with expected file size to be downloaded maybe?

@vagkaratzas
Copy link
Contributor

Awesome! In approval state now, but waiting for the new download module to replace WGET ;)

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.

4 participants