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

Extend charts with more visualizations, adjust interface #2875

Merged
merged 86 commits into from
Oct 17, 2016

Conversation

guerler
Copy link
Contributor

@guerler guerler commented Aug 31, 2016

In this PR, the chart type selection UI has been adjusted to account for a wider range and larger number of possible plugins. Appropriate credit to 3rd party library providers is given in the description of each chart type. This PR improves the handling of misconfigured chart types and missing repository files by displaying proper warnings or providing alternative options. It simplifies the model handling, storage and validation. It also enhances the export/download functionality and quality, particularly for PNGs. The remaining question is if the repository should be local or remote.

Edit: Commit 1805ac8 clones the external repository into Galaxy's plugin directory. It seems to me that thats the best solution for now until viz framework plugins or git repositories can be installed through the toolshed or any other UI.

untitled

untitled1

This repository contains also the BioJS MSA plugin, discussed in #2861. The corresponding repository files are here: https://github.com/guerler/galaxy-charts/tree/master/src/visualizations/biojs/msa.

41be5754-6ade-11e6-8a45-19a713cecae9

The repo now also contains a basic cytoscape viewer. This is a good test case to look at data formats and validation, after all users should be able to use regular tools and benefit from these visualizations. The data format does not seem to be too complicated but if it turns out that users can not easily convert their data to such a json format we might want to avoid components like these. Each visualization should have a wiki page explaining the required data formats. The wrapper files are located at https://github.com/guerler/galaxy-charts/tree/master/src/visualizations/cytoscape/basic.
cytoscape

Another new chart type is the pv protein viewer hosted at: https://github.com/biasmv/pv. It is a PDB viewer. The wrappers are located here: https://github.com/guerler/galaxy-charts/tree/master/src/visualizations/pv/viewer. Screenshot of the editor view:
pdbviewereditor
Screenshot of options view:
customization

Example of output:
pdbviewer

@guerler
Copy link
Contributor Author

guerler commented Sep 30, 2016

Charts is analogous to tool form and tool execution, but limited to js-based visualizations. Just like command-line tools most js-based 3rd party visualization plugins provide many options but no UI. Galaxy can build form-based UIs. Since charts is limited to js, there is not much to do. It consumes the config.js files to build a form with three tabs using regular ui-form libraries. To visualize, charts dynamically requires the visualization's remote wrapper and parses dataset details, a chart model, the deferred process and a list of target html element ids: https://github.com/galaxyproject/galaxy/blob/dev/config/plugins/visualizations/charts/static/client/views/viewport.js#L91. That is the charts part.
The wrapper loads a 3rd-party plugin, configures it using the chart model parameters, and applies it to draw into the html targets. The wrapper accesses Galaxy through the API e.g. to load the dataset content or execute jobs. It may also trigger events in charts by modifying the parsed chart model or rejecting the deferred process: https://github.com/guerler/galaxy-charts/blob/master/src/visualizations/pv/viewer/wrapper.js.

@guerler
Copy link
Contributor Author

guerler commented Oct 8, 2016

I added the drawrnajs plugin from galaxyproject/tools-iuc#958.

drawrnajs

@bgruening
Copy link
Member

@guerler more of this! :)
@yhoogstrate was this based on the latest version of https://github.com/bgruening/galaxytools/tree/master/visualisations/drawrnajs just double checking - and ideally we deprecate my version than.

@guerler
Copy link
Contributor Author

guerler commented Oct 8, 2016

I pulled the node module and touched it up for it to work with webpack. I contacted @Bene200 too. We could update it to a newer version.

@bgruening
Copy link
Member

Awesome!!!

@yhoogstrate
Copy link
Member

@bgruening the one in your repo is Old and had a few Serious render issues with nested loops, So please deprecate it. thanks! @Bene200 has fixed them All over the last year if i remember correctly. @guerler This is seriously awesome!!

@guerler
Copy link
Contributor Author

guerler commented Oct 12, 2016

We have some competing ideas, keep everything together, separate it as it was, extend the toolshed such that it can install git repositories, write a new minimal UI to install them and so on. At this point it seems that the best solution is to keep everything together in Galaxy's plugins directory until plugins can be installed from external sources.

@natefoo
Copy link
Member

natefoo commented Oct 12, 2016

I agree with @jmchilton (and I'm sorry I'm late to this when @guerler has already done the work to separate this from Galaxy), this is relatively core functionality that should remain in Galaxy. In addition, we're trying to move away from the "clone repo" method of deployment and "install things at startup" dependency management, so I think it's a step in the wrong direction to install charts this way and will cause problems for installable Galaxy down the line.

If this was Python we could just package it as a separate python package or if it was Node.js we could make it installable with npm. But this is isn't either and I really don't understand the argument for making it separate (full disclosure, you can put whatever you want in a python package, so it'd be possible to have it in an sdist/wheel, but I think that's the wrong approach here).

@martenson
Copy link
Member

martenson commented Oct 12, 2016

@guerler told me he could probably wrap this into npm module. However given we do not ship node and that this is in fact a viz framework plugin and thus dependent on Galaxy fairly heavily (I think) this makes little sense to me as a long-term solution.

Solution is to have viz plugins hosted and versioned as a TS-repository-like-entity. After SLC admin training I will give a shot to implementing this in the TS.

Until we have a TS-like solution I am +0.9 on having this in the codebase directly.

Given charts behaves like an atomic viz plugin the side-effect of moving it out would remove the functionality that exists by default in Galaxy and could be seen as a regression. Moreover fetching this charts repository via a startup script exposes Galaxy to security issues especially given the size of the javascript fetched and updating.

@jmchilton
Copy link
Member

@martenson So you are going to put viz plugins into the tool shed or charts plugins into the tool shed? Charts plugins are newer obviously - but @guerler has big plans to implement many of them.

@martenson
Copy link
Member

@jmchilton I would like to treat charts as an atomic viz plugin. 'viz plugins' is answer to your question. Having versioned plugins have their own versioned plugins is a problematic architecture imo.

@martenson
Copy link
Member

martenson commented Oct 12, 2016

Ultimately I was hoping for what you call 'charts plugins' to be a many standard viz plugins but with dependency on something called charts (that does not exist yet) thus installable separately. Feasibility? Unknown.

@jmchilton
Copy link
Member

jmchilton commented Oct 12, 2016

@martenson Wasn't the whole concern started when @guerler added some bio stuff to the charts plugin? How does atomically grabbing all chart plugins at the same time address that concern at all? If charts is going to produce many more awesome vizs than the older style plugin over the next N years - implementing a plugin registry for the older-style plugin system seems like a waste. I'd encourage you not to attempt it, but I will try not to get in the way if you do.

Update: This was in response to your first comment, not your second. If feel like implementing the second thing would be a good pre-req to doing the first thing.

@jmchilton
Copy link
Member

...

...

The problem is that as soon as there is a dependency on something called "charts" then there are dependencies. And you have to worry if the version of "charts" is new enough or too new for the chart plugins. Then you are building a package manager again. Which of course you can do - but I'd encourage you not to.

@martenson
Copy link
Member

@jmchilton good point

@jmchilton jmchilton merged commit 1dc2d6a into galaxyproject:dev Oct 17, 2016
@martenson martenson changed the title Link charts to external repo, adjust interface Extend charts with more visualizations, adjust interface Oct 19, 2016
@guerler guerler mentioned this pull request Feb 21, 2017
@guerler guerler deleted the remote_chart_plugins branch February 19, 2020 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants