Add new input to multiqc module for use with --rename_samples

[pinin4fjords]

Supplying a TSV to multiqc with --rename-samples allows you to substitute sample identifiers to allow for consistency across a report.

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:
  • For modules:
    • nf-core modules test <MODULE> --profile docker
    • nf-core modules test <MODULE> --profile singularity
    • nf-core modules test <MODULE> --profile conda
  • For subworkflows:
    • nf-core subworkflows test <SUBWORKFLOW> --profile docker
    • nf-core subworkflows test <SUBWORKFLOW> --profile singularity
    • nf-core subworkflows test <SUBWORKFLOW> --profile conda

[MatthiasZepper]

Given how frequently some form of name cleaning is required, I think it is very useful to have a separate channel to supply the alternative names in a straightforward manner. (In contrast to collecting a Nextflow channel with the sample names into a file, referencing that path in a config file and mixing that file into the extra_multiqc_config channel).

However, I think that the --sample_names argument will be used almost as frequently as --rename_samples.

Hence, I wonder if it would be advisable to change the channel from a path channel to a map like [ val(argument), path(config) ]?

Inside the module, instead of

def replace = replace_names ? "--replace-names ${replace_names}" : ''

we would have something along the line

def replace = sampleMap ? sampleMap.collect { argument, value -> "--${argument} ${value}"}.join(' ') : ''

which would work on an input channel like

def sampleMap = [
    replace_names: '/path/to/sample_aliases.tsv'
]

In that case, when using the MultiQC module, the pipeline authors could .map the path to the argument they wish to use?

Basically what we already did in the rnaseq pipeline for the extra_args, just this time inside the module?

[pinin4fjords]

@MatthiasZepper I'm not convinced, that's not a pattern I've seen elsewhere in nf-core. Can you imagine explaining that in the meta.yml? If we need another file input I think we should have another file input- I'll add it now

[MatthiasZepper]

I'm not convinced, that's not a pattern I've seen elsewhere in nf-core.

But maybe it should become a more common pattern? I agree that this requires a larger discussion, e.g. on the #modules channel prior to merging it into such an important module, but I have always felt that the ext.args pattern is hard to wrap your head around, in particular for end users of the pipelines.

I do not recall how often I have explained how to add extra arguments to a tool on various Slack channels (many times) and the extra_star_align_args & Co. parameters are really popular in the rnaseq pipeline - at least I have seen them quite often being used in param files.

Having a key -value map that is consolidated on a module level with the ext.args could be really neat for pipeline developers to support such extra_[...]_arg pipeline parameters without much headache. It would also help to avoid duplication, if e.g. the same module is used in two different subworkflows that represent two routes of a pipeline. It would become possible to mix in their specific settings into a default map that applies to both.

Can you imagine explaining that in the meta.yml? If we need another file input I think we should have another file input- I'll add it now

I have not yet thought about that problem, but it seems solvable to me. If the code itself is brief and works reliably, it should not be a dealbreaker to describe that functionality schematically.

What worries me more is duplicating the boilerplate consolidation code in every module. I guess that should then be some central function like check_max(). for the configs.

I'll add it now

Fair enough. Hardcoding too many input channels to me also doesn't seem to be desirable, but I guess two additional ones would work.

[pinin4fjords]

Yep, I agree there are things we could improve in the conventions, but that that's a discussion for elsewhere.

Merging for now.