Subworkflow for phylogenetic placement: fasta_newick_epang_gappa

[erikrikarddaniel]

PR checklist

This adds a subworkflow that runs phylogenetic placement with EPA-NG and Gappa.

• 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

[d4straub]

Looks good to me! But this is the first subworkflow that I review, probably a second opinion would be good as well.

[nvnieuwk]

It's preffered to split this channel into multiple input channels that will get merged in the subworkflow. This reduces the complexity for the user too (so you don't have to use arrays in arrays etc)

[erikrikarddaniel]

The problem is that that's difficult to synchronise if the pieces come from different sources, i.e. to make sure that they come in the same order. (At least I don't know how to do that.)

[nvnieuwk]

You can join the channels on meta. Just gotta make sure that all channels that have to be merged do contain exactly the same meta. (You can use the .join() operator for this: https://www.nextflow.io/docs/latest/operator.html#join)

[erikrikarddaniel]

Right. I'd like to give some more context. The workflow is called by my phyloplace pipeline (it's basically the whole pipeline). The sample sheet for the pipeline, parsed with splitCsv(), is turned into a single channel with this structure. If I were to give the subworkflow separate channels, I'd have to split the channel before calling the subworkflow, then join() them in the subworkflow.

I chose the map structure for the channel because I personally prefer to have named inputs when the number of inputs is large.

I plan to include the subworkflow also into ampliseq. I don't foresee any problems calling it with this structure. (Of course it should be documented though. That was an oversight of mine mostly because I'm not so familiar with the documentation format.)

Moreover, I was recently advised to merge channels into one when submitting a module. An advise that I later was grateful for, because I found it more explicit to join() before calling the module rather than the module doing that for me.

[d4straub]

While I agree that the input channel here is a little more complex than for other workflows, I find it quite intuitive due to the naming. I am fine with it as is. @nvnieuwk is there a rule for input channels?

[nvnieuwk]

Aaah okay the context is very clear, thanks! You can leave it as it is :)

[d4straub]

Not sure my suggestions are perfect, but at least to me it makes understanding much easier.

[erikrikarddaniel]

Not sure my suggestions are perfect, but at least to me it makes understanding much easier.

Thank you, much better!

[erikrikarddaniel]

Thank you both @nvnieuwk and @d4straub !