pypgx/runngspipeline

[Jorisvansteenbrugge]

PR checklist

Closes #5987
This PR adds a new module: pypgx/runngspipeline

• 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
• 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 docker

[Jorisvansteenbrugge]

For some reason the conda based tests are not working..

[famosab]

Can you run the test locally with the conda profile?
nf-core modules test pypgx/runngspipeline --profile conda -u

[Jorisvansteenbrugge]

Can you run the test locally with the conda profile? nf-core modules test pypgx/runngspipeline --profile conda -u

@famosab Thank you for taking a look! I get the same error unfortunately.. I wonder if this is the same issue as #4234 opened by @edmundmiller .

I see this error appears when running tests for subworkflows. Could be that we need to add a name to environment.yml? If two modules in a subworkflow have exactly the same environment.yml I suspect the environment names may clash

While this is not a subworkflow, the nf-test for this module is chained with other modules that use the exact same environment.yml. I am however not sure what the solution is.

[famosab]

In theory that should not happen with nf-test anymore as noted down here: #5836

They seemed to have thought about a fix: #6664

Maybe it is worth asking in the slack since this issue came up a few times :)

[Jorisvansteenbrugge]

@famosab Thanks :)

It seems like we are waiting for nextflow-io/nextflow#5489 to get merged. Which should fix the issue (nextflow-io/nextflow#5485)

[LouisLeNezet]

Thanks for the contribution.
I'm just a bit concerned on how the output is given back.
Is there a need for it to be in a zip format ?

[LouisLeNezet]

Is this argument mandatory ?

[Jorisvansteenbrugge]

Hi @LouisLeNezet Thank you for your thorough review, and sorry for replying late! Which argument are you referring to exactly?

[LouisLeNezet]

I was talking about the assembly_version

[Jorisvansteenbrugge]

I have set a default in the process as
def assembly = assembly_version ?: "GRCh38" (https://github.com/nf-core/modules/blob/pypgx/runngspipeline/modules/nf-core/pypgx/runngspipeline/main.nf#L26)

This is in line with how I pass the assembly_version value channel to other processes of the same tool (which are already in master):

• https://github.com/nf-core/modules/blob/master/modules/nf-core/pypgx/createinputvcf/main.nf#L28
• https://github.com/nf-core/modules/blob/master/modules/nf-core/pypgx/computecontrolstatistics/main.nf#L26
• https://github.com/nf-core/modules/blob/master/modules/nf-core/pypgx/preparedepthofcoverage/main.nf#L26

Do you propose something else?

[LouisLeNezet]

I'm not sure that def assembly = assembly_version ?: "GRCh38" is the best thing to do.
I was just wondering if the tool needed this string and if so you could just use: assert assembly : "Assembly name should be provided". From my point of view the specie should not be set as default, but if your previous PR was accepted as such this is fine for me.

[Jorisvansteenbrugge]

Thanks for the contribution. I'm just a bit concerned on how the output is given back. Is there a need for it to be in a zip format ?

@LouisLeNezet
The zip files are simply what the tool outputs, it is not something I added myself

[LouisLeNezet]

Thanks for the contribution. I'm just a bit concerned on how the output is given back. Is there a need for it to be in a zip format ?

@LouisLeNezet The zip files are simply what the tool outputs, it is not something I added myself

That's fine for me then.

[LouisLeNezet]

From looking at the other modules of this tool, this is ready to merge !
LGTM