Add tblastx module #8651
Add tblastx module #8651
Conversation
There was a problem hiding this comment.
Hi Sneha, thank you for contributing to nf-core! While I greatly appreciate your willingness to expand the module library, multiple issues need to be addressed in this pull request.
- Multiple elements required by the module template are missing. I assume you created the module by manually replicating the structure of an existing module rather than using
nf-core modules create. While allowed, this approach requires utmost care that no element of the template is missed, and is thus not recommended. - You have included several non-standard elements, including a license, a README, and a mini-workflow. The license file is superfluous. The README could be helpful, but it is not compatible with the nf-core website. I will need to discuss the mini-workflow with the maintainers, but it should not be used for testing, as nf-test creates its wrapper workflow.
- The script declaration is highly inflexible, as it does not allow the user to pass any custom arguments.
- The test is incorrect, as it does not use nf-test.
- The test files are included in the module directory instead of being linked from the test data repository.
- The module is located at an incorrect path. As a subtool, it should be located at
modules/nf-core/blast/tblastx, inside the already existingblastdirectory.
I highly suggest that you familiarize yourself with:
Then, I suggest the following course of action:
- Close this Pull Request
- Verify which test input files, if any, are already present in the module test data repository, and open a PR to add the missing ones
- Create the module again at the correct path using
nf-core modules create - Open a new PR to add the module to this repository
Thank you once again for joining nf-core contributors. If you have any doubts, don't hesitate to reach out to me here or on Slack.
| @@ -0,0 +1,21 @@ | |||
| MIT License | |||
There was a problem hiding this comment.
You don't need to include a license file in modules. There is an MIT license at the repo root, which correctly covers all code within.
| @@ -0,0 +1,25 @@ | |||
| # blasttblastx | |||
There was a problem hiding this comment.
As of the current template, modules don't contain README files. I will discuss it with other maintainers, but it is unlikely to stand. The documentation for the module should be included in meta.yml to ensure correct webpage generation.
There was a problem hiding this comment.
Agree on behalf of maintainers: the module itself should be relatively self-explantory, and all of this is already ocvered in the meta file
| nextflow.enable.dsl=2 | ||
|
|
||
| process blasttblastx { | ||
| tag "${query.getBaseName()}" |
| tag "${query.getBaseName()}" | ||
|
|
||
| input: | ||
| path query |
There was a problem hiding this comment.
Meta map input and output is missing
| output: | ||
| path "tblastx_results.txt", emit: result | ||
|
|
||
| script: |
There was a problem hiding this comment.
ext.args handling is missing
There was a problem hiding this comment.
All input files for tests should be stored in the test-datasets repository (https://github.com/nf-core/test-datasets) and linked inside the test script.
| @@ -0,0 +1,4 @@ | |||
| params { | |||
There was a problem hiding this comment.
While a config is allowed for tests, input files should be specified inside the test script.
| @@ -0,0 +1,10 @@ | |||
| test_name: default | |||
There was a problem hiding this comment.
This is not the correct format for nf-test (cf. nf-test documentation).
| @@ -0,0 +1,7 @@ | |||
| name: blasttblastx test | |||
There was a problem hiding this comment.
This is not an nf-test snapshot. Other snapshot formats are not accepted.
| -outfmt 6 \\ | ||
| -out tblastx_results.txt | ||
| """ | ||
| } |
| @@ -0,0 +1,25 @@ | |||
| # blasttblastx | |||
There was a problem hiding this comment.
Agree on behalf of maintainers: the module itself should be relatively self-explantory, and all of this is already ocvered in the meta file
| @@ -0,0 +1,34 @@ | |||
| nextflow.enable.dsl=2 | |||
There was a problem hiding this comment.
@snehaag006 did you use nf-core tools to generate teh module tempalte? a lot of this looks very manual written...
|
If it helps, we have a detailed step-by-step tutorial here: nf-co.re/docs/tutorials/nf-core_training/writing-nf-core-modules/chapter-1-introduction :) |
This module adds support for NCBI's BLAST tblastx command, which compares six-frame translations of nucleotide sequences.
Developed by Sneha Agarwal as a contribution to the nf-core modules repository.