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

DrawRNAjs wrapper #958

Closed

Conversation

yhoogstrate
Copy link
Member

Hey everyone,

This PR is a bit different than usual. DrawRNAjs by @Bene200 is a Javascript visualization for RNA 2D structure. Therefore I have the dependencies (css and js files) not managed by conda but dropped them in the static folder of the tool wrapper. Please review this carefully as this is a bit different way of doing it and might initiate a new direction for other contributions sa well.
The reason I built it as a Galaxy tool rather is because I really like the type of visualization and having it in a tool shed gives it much more visibility than only having it in @bgruening visualizations directory at github.

A screenshot:
image

Thanks for looking into this,

Youri

@yhoogstrate yhoogstrate changed the title Drawrnajs complete DrawRNAjs wrapper Sep 22, 2016
@bgruening
Copy link
Member

This is a really nice hack! I like the functionality - I'm a little bit scared how this works, but well done!
@guerler what do you think about this? I tend to merge it, as I think getting Galaxy VIS into workflows will take a few month/years right? So this would be a nice in the meantime ...

ln -s '$__tool_directory__/static/spectrum.css' '${output.files_path}/' ;
ln -s '$__tool_directory__/static/style.css' '${output.files_path}/' ;
ln -s '$__tool_directory__/static/drawrnajs@1.2.6' '${output.files_path}/drawrnajs.js' ;
ln -s '$__tool_directory__/static/galaxy_handshake_utils.js' '${output.files_path}/galaxy_handshake_utils.js' ;
Copy link
Member

Choose a reason for hiding this comment

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

Can you copy these instead? There is no guarantee these directories will remain on the same locations back on the Galaxy web server. Unless Galaxy resolves the symbolic link for you - but I don't think it does.

Copy link
Member

@mvdbeek mvdbeek left a comment

Choose a reason for hiding this comment

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

I think this is very nice, I don't see a reason to not include this.


mkdir -p '${output.files_path}' ;

ln -s '$__tool_directory__/static/bootstrap.css' '${output.files_path}/' ;
Copy link
Member

@mvdbeek mvdbeek Sep 22, 2016

Choose a reason for hiding this comment

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

Wouldn't it be better to copy this over?. If the tool is uninstalled/updated, or the dependency folder moves the visualization will stop working, right?

@jmchilton
Copy link
Member

This isn't best practice - it shouldn't be considered a best practice. I'm fine with you developing tools this way - but can you move it to a different repository? This is supposed to be examples of best practices.

There is so much static JS there that needs to live with the tool forever - if there are security problems down the road it is a mess for admins to deal with that - and in fact you have no way of conveying it to the the admin.

@bgruening
Copy link
Member

Oh damn it, I thought this PR was against galaxytools. Sorry I need to correct me, I would be fine to merge this into galaxytools not here. @jmchilton is completely correct this should not be part of IUC.

@guerler
Copy link
Contributor

guerler commented Sep 22, 2016

This is purely js-based right? I dont want to hold this PR up, but I think it would be a great candidate to be added as charts plugin, we have a couple examples at galaxyproject/galaxy#2875. If you are interested I suggest to take a look at the PDB viewer integration, this one should work in a very similar fashion.

@bgruening
Copy link
Member

@guerler thought so. I will start with all the plugin in galaxytools. But the use-case here is that it would be nice to run these tools in the end of a workflow. Is this planned or already possible?

@guerler
Copy link
Contributor

guerler commented Sep 22, 2016

I see. Not yet but its something we could work on if there is enough interest/priority. In fact this PR demonstrates how one could approach it.

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.

5 participants