At the risk of opening this up to too many eyeballs, but I think the below question is better suited to a more general audience: I am developing a module that constructs a piped samtools command - i.e. you pass it in a set of SAM/BAM/CRAM files and a list of commands, e.g.

["view", "sort"]

and it will construct a command that pipes the output of

samtools view

into

samtools sort

and then write an output BAM file, saving space on intermediate outputs. Currently I've written this so that it also supports

samtools fasta

and

samtools fastq

as outputs, so you could pass in some BAMs, process them, and get the reads. But this adds quite a lot of complexity to the module, which will make maintenance more difficult. Is this something that folk would generally want to use in a module like this, or would you be happy to just fire the output into a separate

samtools/fastq

process if needed? github.com/nf-core/modules/pull/11550

This is very cool imo. I think this is useful, and not having this in the piped command seems to defeat the purpose a bit? Have you considered having another input (e.g.

outfmt

) that can be

bam

fastq

or

fasta

with e.g. bam as a default, to make this a bit more explicit, and also a bit more obvious?

The module already only supports a subset of samtools commands - those that read and write *AM. Other commands (eg.

samtools stats

) are already not supported, with the mind that such tools should be run separately on the written file. This vastly simplifies the logic as a lot of samtools tools do not have a completely consistent interface. Removing fast{a,q} would follow this logic as these also "non-standard" end-points, and could not be included in the middle of a pipeline chain. I included them for the moment as they seemed they would be used more frequently and hence be worth the complexity, though. > Have you considered having another input (e.g.

outfmt

) that can be

bam

fastq

or

fasta

with e.g. bam as a default, to make this a bit more explicit, and also a bit more obvious? None of the other samtools modules operate this way, which would be inconsistent IMO

None of the other samtools modules operate this way, which would be inconsistent IMO (edited)

Well, samtools sort has index type for instance, which is a value channel to determine the index type if present. I'd happily make them some of them take an input channel that is

bam/cram/sam

to determine the output format. But I don't think that you need to be worried about consistency with other modules with this one in particular

Removing fast{a,q} would follow this logic as these also "non-standard" end-points, and could not be included in the middle of a pipeline chain. I included them for the moment as they seemed they would be used more frequently and hence be worth the complexity, though.

Yes. Given that they cannot be part of the pipeline anyway, I thought controlling the output/endpoint in a different way might be clearer. This is mainly driven by my assumption that I would be annoyed by a module that allows me to construct a samtools pipeline, but then still needs to go into another samtools tool / module to format.

Not currently! It would be simple to include that check though.

You could, but I would prefer my earlier suggestion 😉 since that way you would not need that check, and would also make it more difficult to accidently construct a broken pipeline.

My concern would be that your suggestion would add additional complexity to the code, and I'm receiving a lot of comments about the maintainability already

In my mind not having

fastq

or

fasta

as valid commands should simplify the

get_file_output

section?

I'm not sure it would, especially if using the approach of appending it to the pipeline list

I have taken another look at your proposal, and also at the historic context and have another comment :D I am aware that this is unconventional, but have you considered to simply create a module that takes a userdefined command, i.e. provides an environment but the script section is essentially

$command

? This would require the module user to craft the correct command, but I would argue that someone who is able to correctly piece together samtools subcommands into a pipechain is probably also able to add in

-@ ${task.cpu}

?

My feeling is that that would be more annoying to specify in general - at that point a user might as well specify their own local module directly!

Yeah, fair point, it is essentially specifying your own module without having to put the boilerplate around it.

But in a truly generic module, you wouldn’t know what to stage / channel out

The truly generic module would have to stage out every possible extension as an optional output, I guess?

I'm not sure I follow what you mean

I will need a bit to actually figure out how to translate what you are doing into what I mean, but the basic Idea would be e.g to replace github.com/prototaxites/modules/blob/…/main.nf#…:

commandmap = [
fasta: "fasta" + "-0 ${prefix}_other.fasta.gz -1 ${prefix}_1.fasta.gz " + (single_end ?  "-2 ${prefix}_2.fasta.gz" : "") + "-s ${prefix}_singleton.fasta.gz"
]

and then using

commandmap.get("fasta")

That's an interesting idea, but it doesn't solve the problem of how to construct the command itself, which depends on whether it's the first command, an intermediate command, or an output command

Whats stopping you from putting the current logic into the same map?

It would have to be a map-of-maps, I think

Hm, ok, theres probably parts of the current approach that I do not get

You could get clever with it - a "default" map that gets overwritten with exceptions, maybe

i think if you do a lot of ternary ops in the map it should be ok? But yes, it will remain complex, the main point would be that it is more obvious how the pipeline command for each of the valid_commands would be constructed