https://nf-co.re logo
Join Slack
Powered by
Hi, I'm wondering if other people would be interes...
# nft-plugins
l
Hi, I'm wondering if other people would be interested to add some new functionalities to the sanitizeOutput function of nft-utils ? I was thinking to add the following: • fix the recursive parse with unstable keys (give only the top folder as for now) • add vcf and bam md5 (something like: sanitizeOutput(process.out, vcfKeys:["my_vcf"])) • ignore file pattern (to remove some extension but still get md5 for other file when checking a folder)
n
Hi these sound like solid ideas! Feel free to implement them in the function 🙂. I only think that the second suggestion won't work as this depends on code from other plugins. But feel free to try 😛
l
I will try 😉
🎉 1
n
Feel free to ask my review when you have something!
👍 1
s
Can you add checking that the unstable keys actually exist 😉
👍 1
l
Here is the first fix. I will try to do things in small PRs to keep it easy to review. github.com/nf-core/nft-utils/pull/74
Here is the second one for ignoring keys and adding a key name check. github.com/nf-core/nft-utils/pull/75
n
I reviewed 😉
l
Thanks !
@Simon Pearce: When you say
add checking that the unstable keys actually exist
Would you like an error or a warning ? Do you have a specific use case in mind ?
s
probably an error
I can't think of any case where you'd want to be able to set unstableKeys to something that wasn't an output of the process
(or workflow)
l
That's what I did for the moment. Nicolas was wondering if a warning would be better. On my side I should prefer to be a bit more stringent.
s
I've done it a couple of times in the migration where I've put the wrong name
and then nf-test passes, but the channel is still not stable
l
Okay, make sense to me. Could you review the PR, therefore ? github.com/nf-core/nft-utils/pull/75
👀 1
m
Nice
Let me know when you need a release
l
Can I get an approval on github.com/nf-core/nft-utils/pull/75 so I can continue on vcf and bam support ?
👀 1
m
What was Simon reply on the comment from nicolas?
l
It is just above. He is in favor for an error
✅ 1
Hi, A new PR is ready to add md5Reads and md5Variants for bam and vcf. It uses nft-bam and nft-vcf to compute those. I've added test with and without the plugin installed. What do you think ? github.com/nf-core/nft-utils/pull/76
👀 1
n
I left some comments, thanks for this!
l
I've updated the PR to fix the different comment. CRAM is now supported but you'll need to provide fasta and fai.
m
fai too? I thought only fasta was necessary
l
The fai needs to be in the same folder. If absent, it crash and tells you it couldn't compute the reads md5.
m
what now?
l
Okay, I might have found a easier solution than using the class. I can use the AlignmentFile method instead of the class. The cram() method is downloading the fai under the hood.
party this 1
m
oh I see
n
I like this new approach! Only one comment left about the name but looks good otherwise
l
I've updated the doc, but I don't see many alternative to vcfMD5Keys and bamMD5Keys. Do you have a specific idea ?
n
mappedMD5Keys
maybe?
uhm yeah it's hard to name something for this 😅
l
I have to admit I prefer 
bamMD5Keys
or maybe 
readsMD5Keys
?
s
bamKeys
?
l
> bamKeys It was my first idea 😅, but Nicolas though it didn't reflect enough what would happen to these keys.
s
and just drop the MD5 part
n
Ah I'm not going to complain about this any longer 😛 it's just a name
l
Would 
readsMD5Keys
works, as it is in the end the real important function used from nft-bam ? I can use 
variantsMD5Keys
for the vcf similarly ?
n
I like that one!
l
Done
🙌 1
Can I get an approval then ?
👀 1
✅ 1
n
Done, sorry I got distracted
t
Hi @Louis Le Nézet, Currently I am checking header of bam file, which need getHeader, then removing UR and CL (abs path) before getting snapshots (here). I wonder if this could be supported?
n
That might be a bit too custom for this function I fear
❤️ 1
l
Hi @Thi Hong Hanh Hoang, The aim of the 
sanitizeOutput
from nft-utils and of the different addition I'm making is to provide a one line assertion for most of the situation encountered in nf-core modules repository. For bam file, I guess that getting a snapshot of all the reads is sufficient for our context. However, this could be added to nft-bam, if you really need to test for header stability.
n
This already exists in nft-bam, I guess you are already using that functionality to test the header?
5 Views
Open in Slack
PreviousNext
Powered by