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 XML visualizations based on crossbio #61

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

Conversation

bgruening
Copy link
Contributor

This PR will add a Galaxy visualization for BLAST XML files based on http://crossb.io/ and developed by @greenify.

Should fix: #60

@peterjc please not that I have trouble to bind this visualization to a Tool Shed datatype. See more as an inline comment.
You can test this by simply copy this folder into GALAXY_ROOT/config/plugins/visualizations/.

<data_sources>
<data_source>
<model_class>HistoryDatasetAssociation</model_class>
<test type="isinstance" test_attr="datatype" result_type="datatype">blast.BlastXml</test>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterjc can you confirm that this is not working for you? If you change it to xml.GenericXml or data.Textit should work.
Another reason to move the BLAST datatypes into the core?

@peterjc
Copy link
Owner

peterjc commented May 14, 2015

It could be clearer crossbio/crossbio#2 but crossbio is also using the MIT licence, so that isn't a problem for adding this :)

Right now TravisCI is failing with AttributeError: 'NoneType' object has no attribute 'endswith' but that looks like a Galaxy problem (maybe we shouldn't track the dev branch?)

Right now I believe the Galaxy Tool Shed still doesn't support visualizations - so how would any end user install this? Just copy the folder under GALAXY_ROOT/config/plugins/visualizations/ as you suggested for me to test this?

@bgruening
Copy link
Contributor Author

Right now I believe the Galaxy Tool Shed still doesn't support visualizations - so how would any end user install this? Just copy the folder under GALAXY_ROOT/config/plugins/visualizations/ as you suggested for me to test this?

Yes, this is the only option I know.
Tracking the next-stable branch seems to be better to me.

@jmchilton
Copy link
Contributor

so how would any end user install this

Easy - put it in core :). I would 👍 a PR to core especially if it also came with the blast datatypes - not sure everyone else would though obviously.

It would also mean we can correct backward incompatible changes to the framework like galaxyproject/galaxy#243 by modifying the plugin itself to match the version of Galaxy it is stored with.

@bgruening
Copy link
Contributor Author

@jmchilton you mean the visualization into core? We would need the datatypes as well to make it work :)

Somehow I missed #243. @peterjc if you use latest :dev you should adopt this before you test it.

@jmchilton
Copy link
Contributor

@bgruening Yes - I meant the visualization.

@peterjc
Copy link
Owner

peterjc commented May 14, 2015

If the Galaxy dev / IUC consensus is to move datatypes into the core, we can put the BLAST datatypes back again - which would allow putting this visualization into the Galaxy core too. That seems pragmatic since for now the Tool Shed doesn't support visualizations.

[@greenify this code looks interesting, I want to see it merged here or into the Galaxy core - just unclear where is the best fit right now]

P.S. Looking at galaxyproject/galaxy#243 now...

@bgruening
Copy link
Contributor Author

I think the BLAST datatypes are pretty safe to move to core now. They have a proper definition in blast.py, are used a lot in other tools and have sniffers, even split/merge functions.

If you target dev I can patch the changes for galaxyproject/galaxy#243 in.

@wilzbach
Copy link

It could be clearer crossbio/crossbio#2 but crossbio is also using the MIT licence, so that isn't a problem for adding this :)

@peterjc: thanks for bringing up this issue. I will add a proper license file tomorrow once I get back from my short vacation!

It's fantastic that you guys even consider to move this to your core :)

@peterjc
Copy link
Owner

peterjc commented May 14, 2015

@greenify Adding the code to this repository (galaxy_blast) is under my control, and given @bgruening's endorsement I am positive about this (although I have yet to test it).

Adding it to the main Galaxy core is a bigger hurdle, but has respected advocates pushing for it :)

@peterjc peterjc mentioned this pull request May 21, 2015
13 tasks
@carlfeberhard
Copy link

If that datatype test is erroring or failing on a TS installed datatype - that's a bug. I'll look into that.

@carlfeberhard
Copy link

@bgruening @peterjc re: the config file datatype: if you go to '/api/datatypes/mapping' and search for your BlastXml datatype in class_to_classes, what is the key used? blast.BlastXml or something else? Try that value inside the <test> element:
<test type="isinstance" test_attr="datatype" result_type="datatype">...the key used...</test>

I've installed the devteam blast types from the toolshed and gotten a visualization to show using the above technique.

@bgruening
Copy link
Contributor Author

@carlfeberhard thanks for looking into it. For me this is: BlastXml.BlastXml and with this it is working. How comes that we have a different id?

@carlfeberhard
Copy link

Ah - no, sorry: miscommunication. For me the id for the devteam blast xml datatype was BlastXml.BlastXml as well. I just didn't include that in my question to prevent confusion or in case Peter's datatype had a different value.

So it's working with that? If so, good!

@bgruening
Copy link
Contributor Author

Yes, it's working and it looks great!
Just one question, why? ;) I thought it's always the module.Class schema.

@carlfeberhard
Copy link

The way the registry converts class names to classes using the datatypes registry is less than ideal. It's something I'm working on.

@bgruening
Copy link
Contributor Author

Ok! Sorry, for the trouble.

carlfeberhard and others added 2 commits October 14, 2015 11:23
Fix datatype on config to BlastXml.BlastXml, add alternate test using…
@bgruening
Copy link
Contributor Author

@peterjc can me merge this here? So it get's some attention.
Do you plan to merge the datatypes into core?

@peterjc
Copy link
Owner

peterjc commented Oct 15, 2015

One of the TravisCI tests was failing, rerunning fixed it so it was probably just an SQLite locking issue.

@bgruening If you're happy to be the contact person for this, then sure - I can merge it. It might be time to move this repository to a team model, perhaps under https://github.com/galaxyproject/ like the IUC owned repositories?

If the Galaxy team agree with pulling datatypes back into the core (after previously moving them to the Tool Shed), then I'm happy to help with that. [Update: That has been done now]

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.

BLAST visualizations
5 participants