bgruening / galaxytools

:microscope::books: Galaxy Tool wrappers
MIT License
115 stars 222 forks source link

hifiasm: expose `--trio-dual` parameter and parental read list inputs for trio mode #1339

Closed abueg closed 9 months ago

abueg commented 9 months ago

hello!! 👋🏼

i have made edits to the hifiasm wrapper to expose options that have been available, just not available in the wrapper yet: 1) the --trio-dual parameter, which should only be an option when running in trio mode 2) the -3 and -4 input options, which allow the user to provide partitioned read lists as parental information, instead of the parental raw data. to add this option i restructured the tool somewhat so that now there is a reads-vs-list conditional inside the trio mode. i've restricted the list inputs to be one file only; primarily because i can't really imagine a scenario where one would end up generating multiple read list files, but also because since these are just plain text files of FASTQ header names, it should be easy for the user to just concatenate them if need be

i've checked out the various options using planemo serve and things look to be in working order... please have a look when you have a chance 🚀

abueg commented 9 months ago

EEP did not update the trio test case to select reads, my bad, will fix

bgruening commented 9 months ago

@abueg do you have a test (file) for the new list functionality?

bgruening commented 9 months ago

We could also argue that if you have this file with header names it should be easy to construct fastq files base on the headers and keep the functionality out of this tool. So just recommend users to construct fastq files externally.

bgruening commented 9 months ago

The PR looks great to me, besides the missing test and the theoretical UX question. Thanks!

abueg commented 9 months ago

@abueg do you have a test (file) for the new list functionality?

will make one now 👍🏼

We could also argue that if you have this file with header names it should be easy to construct fastq files base on the headers and keep the functionality out of this tool. So just recommend users to construct fastq files externally.

oh wait, sorry, i was unclear in describing the list files -- they're lists of fastq headers for the CHILD reads, saying that abc read should be maternal, xyz read should be paternal, and so forth. so even if you split the child reads into those binned fastq, that doesn't work as an input to hifiasm, you have to give it the whole child reads and then these lists so it knows how to partition them. the list files aren't for editing the parental files, sorry if i made it sound like that!

it's a useful feature for when there's some mis-binning after the usual process (yak count -> hifiasm trio, which does the binning using those yak counts), because it bypasses the yak count step and tells hifiasm explicitly how to bin things.... does that make sense :face_with_spiral_eyes:

bgruening commented 9 months ago

Ah I see. Thanks for explaining!!!

abueg commented 9 months ago

ahhh S.O.S. @bgruening , sorry i'm not sure what needs to be changed for the test... so i have the test right now set up to check the log to make sure it says "read n reads from the list" to make sure the list input is working, but i think maybe i am specifying the test parameters incorrectly? they look like the way the other trio test is structured, though, so i am a bit confused....

image
bgruening commented 9 months ago

Looks like tests are passing! It will be installed during the weekend.

abueg commented 9 months ago

YAAAY thank you so much!!