-
Notifications
You must be signed in to change notification settings - Fork 478
add COMEBin utility script #7285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This tool doesnt work yet since the recipe isnt updated yet. I will restart the test when it was merged: bioconda/bioconda-recipes#59392 |
I also dont know why the linting is failing since at there is no error when i run it locally with planemo. The only difference which i did see in the GitHub logs is this:
Since i didnt change comebin this make no senee that this in an error and comebin_bam is new so there should be no version for it? |
Also there is an option to set RAM for this Tool. i know there is a galaxy variable to use but i didnt found anything to it how to use and there are not really good example in any wrapper uploaded in iuc. So can i only set it it in the command line or do i need specify a number such that it have this certain number of RAM? |
…s-iuc into comebin_script
okay now only the linting problem is there. I think it is the quoted error which keep it failing. Otherwise i couldnt found it in the logs why the linting is erroring |
@bgruening here is the problem as well which was in #7263 |
@SantaMcCloud Its green now! :) |
oh okay nice then thank you :) |
</output> | ||
</test> | ||
</tests> | ||
<help> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help is a bit sparse :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this bash script doing? Why is bowtie2 not good enough? This looks a bit suspicious to me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did change the help to answer this question.
This script was build using a modified version of the binning.sh of MetaWRAP and i assume the tool itself was build to fit to the these generated BAM files which means that other could work but i will not bet on this. Since BAM files are binary it is hard to see the difference but i could link you a history where 2 Comebin runs did run 1 with the BAM file generated by the the utitlity script and one with the Bowtie2 output: https://usegalaxy.eu/u/santinof/h/metagenome-assembled-genomes-mags-generation-imported-from-uploaded-file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SantaMcCloud I still feel there is still room to update the help text. As of now, it lacks quite a lot of information. Maybe you could add what does the script does? Why is this script preferred over Bowtie2? What are inputs, expected outputs? That would be much helpful for the users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added the help section a little bit. There si not a lot left what i could write there since this scritp use case is only the preprocessing for COMEBin. Bowtie2 doesnt work here and this script output doesnt work for other binnner. If this still is not enought since i can not write more detailed since i dont know metaWRAP i have to read a bit into it.
Co-authored-by: Saim Momin <64724322+SaimMomin12@users.noreply.github.com>
Co-authored-by: Saim Momin <64724322+SaimMomin12@users.noreply.github.com>
@SaimMomin12 could you review it again? The test did error because of the CL all should be fine! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor label corrections
Thanks did commit them @SaimMomin12! :) |
FOR CONTRIBUTOR: