Skip to content

humann3#1679

Closed
kohjy-ag wants to merge 2 commits intomasterfrom
humann3
Closed

humann3#1679
kohjy-ag wants to merge 2 commits intomasterfrom
humann3

Conversation

@kohjy-ag
Copy link
Copy Markdown

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:
    • PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
    • PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
    • PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware

@kohjy-ag kohjy-ag changed the title humann8 humann3 May 20, 2022
Comment thread modules/humann/main.nf
@@ -0,0 +1,76 @@
// TODO nf-core: If in doubt look at other nf-core/modules to see how we are doing things! :)
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.

Remove the TODO statements everywhere (in meta.yml and modules/humann/main.nf)

Comment thread modules/humann/main.nf
output:
// TODO nf-core: Named file extensions MUST be emitted for ALL output channels
tuple val(meta), path("outdir"), emit: outdir
// TODO nf-core: List additional required output channels/values here
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.

Align the emit statements, e.g.:

tuple val(meta), path("outdir")  , emit: outdir
path "versions.yml"              , emit: versions

Comment thread modules/humann/main.nf
// TODO nf-core: Please indent the command appropriately (4 spaces!!) to help with readability ;)
"""
humann \\
-o outdir \\
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.

I would suggest setting the outdir as a custom name per sample (e.g. ${prefix}_outdir). If you would use the module to analyze multiple samples in a pipeline, the results would overwrite each other. You should also change the output channel to: tuple val(meta), path("*_outdir"), emit: outdir if you were to do this.

Comment thread modules/humann/meta.yml
@@ -0,0 +1,51 @@
name: "humann"
## TODO nf-core: Add a description of the module and list keywords
description: write your description here
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.

Don't forget to write a short description of the tool :p


input = [
[ id:'mpa_v30_CHOCOPhlAn_201901' ], // meta map
file('https://raw.githubusercontent.com/nf-core/test-datasets/sarek/testdata/umi-dna/qiaseq/SRR7545951-small_1.fastq.gz', checkIfExists: true),
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.

I would suggest using the params.test_data['homo_sapiens']['illumina']['test_1_fastq_gz'] here instead of the full link. You can find all test datasets for modules either on https://github.com/nf-core/test-datasets/tree/modules or in the conf/test_data.config

input = [
[ id:'mpa_v30_CHOCOPhlAn_201901' ], // meta map
file('https://raw.githubusercontent.com/nf-core/test-datasets/sarek/testdata/umi-dna/qiaseq/SRR7545951-small_1.fastq.gz', checkIfExists: true),
file('chocophlan', checkIfExists: true),
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.

What are these files meant to be?

tags:
- humann
files:
- path: output/humann/versions.yml
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.

Remove versions.yml from the file, if you update to the latest version of nf-core you won't have to delete these manually (you can also see that you are not getting any other output, is this supposed to happen?)

Comment thread modules/humann/meta.yml
Groovy Map containing sample information
e.g. [ id:'test', single_end:false ]
#
## TODO nf-core: Delete / customise this example input
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.

Update your input fields to correspond with the actual input of the module :)

Comment thread modules/humann/meta.yml
description: File containing software versions
pattern: "versions.yml"
## TODO nf-core: Delete / customise this example output
- bam:
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.

Also update your output field to correspond with the real output :)

@nvnieuwk
Copy link
Copy Markdown
Contributor

You can also see there are some trailing whitespaces detected by the Code Linting / EditorConfig check. These should be removed too :)

@muffato muffato added new module Adding a new module WIP Work in progress labels Sep 27, 2022
@JoseEspinosa
Copy link
Copy Markdown
Member

Hi @kohjy-ag , thank you for this PR! We are merging as many modules as possible now due to and impending restructuring of the entire repo. This will mean you will need to update the module to reflect these changes before it can be merged in the future. It appears like this module isn't ready to be merged so we are converting it to draft and adding the WIP label. If this isn't the case please let us know and we will try to get the module in before the changes. Thanks again!

@JoseEspinosa JoseEspinosa marked this pull request as draft September 28, 2022 08:50
@matthdsm matthdsm added the stale Stale label Mar 7, 2023
@matthdsm
Copy link
Copy Markdown
Contributor

matthdsm commented Mar 7, 2023

Hi there!

We've noticed there hasn't been much activity here. Are you still planning on working on this?
If not, you can ignore this message and we'll close your PR in about 2 weeks

Cheers
the nf-core maintainers

@matthdsm matthdsm closed this Mar 22, 2023
@maxulysse maxulysse deleted the humann3 branch September 18, 2023 16:29
@vinisalazar vinisalazar mentioned this pull request Apr 15, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new module Adding a new module stale Stale WIP Work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants