Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

cecilpert
Copy link
Contributor

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 ?

@cecilpert
Copy link
Contributor Author

Ajout également des sélecteurs dans prokka.ml pour pouvoir réutiliser les fichiers de sortie

Copy link
Owner

@pveber pveber left a 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 file Blast


let db_name = "db"

let fastadb fa dbtype =
Copy link
Owner

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

Copy link
Owner

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 ;
Copy link
Owner

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]

Copy link
Contributor Author

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)

Copy link
Owner

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) ;
Copy link
Owner

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?

Copy link
Contributor Author

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 ?

Copy link
Owner

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.

Copy link
Owner

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.

@cecilpert
Copy link
Contributor Author

I tried to create two different PR for blast and prokka but it fails, I don't know how to do it.

@pveber
Copy link
Owner

pveber commented Dec 6, 2017

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.
@pveber pveber force-pushed the master branch 3 times, most recently from fa83704 to 49c7a0e Compare July 6, 2022 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants