feat: allow bwa-mem to output sam format#11457
feat: allow bwa-mem to output sam format#11457piplus2 wants to merge 8 commits intonf-core:masterfrom
Conversation
|
There's a possible issue with the subworkflow |
|
I think the CI fail because of this bug #11519 |
|
We usually want to have compressed output so having |
|
The current code shows an inconsistency: when task.ext.args2 = '--output-fmt sam' it never returns the sam file. So this commit makes the code work as expected. If we want to exclude the sam, then it should be forbidden in the args2 or at least there should be a feedback about this behaviour. |
|
That is a fair point! I think then we need to solve the other bug and add a test for this new output file and we should be good to go. |
|
I'm working on the fix of the subworkflow that makes the CI fail. |
7404fd2 to
ccf7952
Compare
ccf7952 to
e8e3e1b
Compare
|
I'm updating the snapshot to match the modified test, which expects the sam output too now. |
There was a problem hiding this comment.
Can you add a test where you actually expect the sam file in the output please? :)
There was a problem hiding this comment.
I've just added process.out.sam, do you think it may be worth writing a specific test for the sam only? I did not include it to match the cram, csi and crai.
There was a problem hiding this comment.
in theory we should test each output properly. So yes please add it :) The way its done for nw we can already see that sam is not created when it should not be created (which is good).
There was a problem hiding this comment.
I've added 2 tests (single and paired read) to check that we actually get a sam output.
2e4c51d to
c8e4f0a
Compare
There was a problem hiding this comment.
We want the ext.args to be present in the main.nf.test file. That makes everything more readable in one go. See the module specifications for more info.
Other than that good job 🥳
There was a problem hiding this comment.
Oh sorry, my bad! Fixing it now!
There was a problem hiding this comment.
It looks all fine now.
0a900f8 to
17a2c56
Compare
17a2c56 to
e04edc0
Compare
PR checklist
Closes #11456
topic: versions- See version_topicslabelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile condanf-core subworkflows test <SUBWORKFLOW> --profile dockernf-core subworkflows test <SUBWORKFLOW> --profile singularitynf-core subworkflows test <SUBWORKFLOW> --profile condaThe patch allows BWA/MEM to produce SAM format outputs when
task.ext.args2 = '--output-fmt sam'. In that case, we can skip samtools view as bwa mem already produces that format.