czbiohub-sf / nf-predictorthologs

*de novo* orthologous gene predictions from bam + bed or fasta/fastq data
MIT License
4 stars 2 forks source link

tokenized ksizes for sencha translate and sencha index #69

Closed phoenixAja closed 4 years ago

phoenixAja commented 4 years ago

nf-core/predictorthologs pull request

i was getting some errors when running on the bat data because the ksizes weren't being properly tokenized and were instead being processed as strings, this is my fix. I updated test_sambamba config to test the new input peptide ksize and params.translate_peptide_molecule.

closes issue #67

olgabot commented 4 years ago

Oh awesome thank you so much! I just filed an issue earlier (https://github.com/czbiohub/nf-predictorthologs/issues/67) and was gonna look at it tomorrow but you beat me to it! Would you be able to tokenize the --translate_molecule, too?

phoenixAja commented 4 years ago

@olgabot sure!

phoenixAja commented 4 years ago

i'll do it on this PR

phoenixAja commented 4 years ago

cool i think --translate_peptide_molecule should be good to go now as well 👍 , that one was a lot easier. Maybe we should make another test file for sencha_translate and sencha_index, or maybe it's good enough to just sprinkle in to other test configs?

olgabot commented 4 years ago

cool i think --translate_peptide_molecule should be good to go now as well 👍 , that one was a lot easier. 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

Maybe we should make another test file for sencha_translate and sencha_index, or maybe it's good enough to just sprinkle in to other test configs?

Yeah that's a great idea! Could you add one, like test_multiple_ksizes_molecules or something?

olgabot commented 4 years ago

@lekhakaranam this will solve your error, too!

olgabot commented 4 years ago

This PR moves the test_download_refseq test into a separate file allowing for failure and may help with getting the tests to pass faster: https://github.com/czbiohub/nf-predictorthologs/pull/66

phoenixAja commented 4 years ago

This PR moves the test_download_refseq test into a separate file allowing for failure and may help with getting the tests to pass faster: #66

ok i can pull that PR into this one to see if that helps

phoenixAja commented 4 years ago

@olgabot can i merge this in? It looks like the tests have stalled, i tested all the ones stalled locally and they all pass

olgabot commented 4 years ago

yes please go ahead!