-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add blast.ml to lib/bioinfo #38
base: master
Are you sure you want to change the base?
Conversation
…an unique prefix name for results and recover them more easily
Ajout également des sélecteurs dans prokka.ml pour pouvoir réutiliser les fichiers de sortie |
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.
This is a great start! In addition to the comments below, here are two other important points:
- remove modifications from
prokka
as they are not related to the current PR - add an
mli
fileBlast
|
||
let db_name = "db" | ||
|
||
let fastadb fa dbtype = |
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.
better call it makedb
since it's the name of the tool
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.
also dbtype
should be the first argument and named
mkdir_p dest ; | ||
cmd ~env "makeblastdb" [ | ||
opt "-in" dep fa ; | ||
opt "-dbtype" ident dbtype ; |
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 type of dbtype
is Bistro.Template.t
, which is really not convenient for a user. I'd much prefer have a polymorphic variant type here [
nucl | prot]
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.
So we have to create the type ? In the .ml ? I don't really see how to call the function then.
Something like type polymorphic_variant = Prot | Nucl
in .ml and call Blast.makedb ~dbtype:Prot ref_prot
outside ? With a "reconversion" to string somewhere to call the command line.
(it's not exactly like that because it's not working)
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.
actually if you use polymorphic variants `Nucl
and `Prot
, you don't need to declare the type and it's also easier when calling the function. So yes, basically a call would be just like you say Blast.makedb ~dbtype:`Prot ref_prot
, and yes you have to provide the conversion function that will take a variant and make an appropriate Bistro.Template.t
out of it.
cmd "blastn" ~env [ | ||
opt "-db" ident (dep db // db_name) ; | ||
opt "-query" dep query ; | ||
opt "-out" ident (dest // out_name) ; |
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's the motivation for creating a directory here? Couldn't we give dest
to the -out
option?
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.
Because it's already created in makedb ? Or do we have to create it in the main pipeline ?
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.
Well yes, the same could be said about the database, I think in both cases you don't really need to create a directory and put the resulting file in it. The resulting file should be put at the location dest
directly I think.
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.
To be more precise in think your wrapper could be simplified like this:
opt "-db" dep db ;
opt "-out" ident dest ;
of course the wrapper for makedb
should be modified accordingly.
I tried to create two different PR for blast and prokka but it fails, I don't know how to do it. |
You should create one branch per topic, one for blast the other for the fix on prokka, push both branches to your github depo, and create a new pull request from each. |
…to have an unique prefix name for results and recover them more easily" This reverts commit 1f01944.
fa83704
to
49c7a0e
Compare
Bonjour,
Voici le script .ml pour blast. Il y a uniquement blastn et blastp pour le moment, et toutes les options n'ont pas été mises, je ne sais pas si il faut toutes les ajouter ?