-
Notifications
You must be signed in to change notification settings - Fork 448
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
A completely refurbished taxonomy tools collection #463
Conversation
…ng devteam taxonomy tool_collection. devteam tools will be put into legacy folder. These tools can be used with blast, diamond, and kraken and visualize directly in krona using iuc's krona wrapper.
@nekrut if you give me commit access to your repository I will do my review on the fly. For example adding sha256sum is annoying and I can help with this. |
@@ -0,0 +1,38 @@ | |||
<?xml version="1.0"?> | |||
<tool_dependency> | |||
<package name="ghostscript" version="9.10"> |
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 file needs to be properly indented.
@bgruening : I gave you push access to my repo |
…nto taxonomy_tools
@bgruening @nsoranzo @peterjc pls take a look |
@@ -0,0 +1,96 @@ | |||
<tool id="lca1" name="Find lowest diagnostic rank (LCA)" version="1.2.0"> | |||
<description>in a taxonomy</description> | |||
<command interpreter="python"> |
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 tool uses sort
to we want to depend on gnu-coreutils?
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 set LC_ALL=C
for reproducibility?
</command> | ||
<inputs> | ||
<param name="input1" format="tabular" type="data" label="Input File"/> | ||
<param name="input2" type="integer" value="2" label="First Column"/> |
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.
@nekrut any reason to not use data_column here?
|
||
Drawing the tree with default parameters (without changing anything in the interface) will produce this tree: | ||
|
||
.. image:: https://galaxyproject.org/tool_images/taxonomy/t2ps_ideal.png |
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.
local images?
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.
My main problem with local images is that they just don't work for local testing. They may work once the tools migrates to toolshed. So I would like to keep the URLs for now
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.
Yeah, this should be fixed in Galaxy and planemo.
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 for now, let me have them as remote URLs. They are also committed to galaxy ansible playbook
Should we include all images locally or are web-urls ok? |
I think that embedding remote images is fine if they are part of the upstream documentation, but in this case I'd vote to include them in the repository. |
tool (aka Taxonomy format) as the input. | ||
name: lca_wrapper | ||
owner: iuc | ||
remote_repository_url: https://github.com/galaxyproject/tools-iuc/tree/master/packages/tool_collections/taxonomy/lca_wrapper |
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.
/packages/tool_collections/
-> /tool_collections/
…nto taxonomy_tools
…into taxonomy_tools
@bgruening @nsoranzo @yhoogstrate @peterjc pls, take a look and merge if you don't have major objections |
Please don't merge! Sorry Anton, here is something wrong with the merge you removed packages. |
Yes. I see. Probably I did something stupid. |
closing in favor of #501 |
This completely replaces devteam's taxonomy tool_collection. Devteam tools will be put into legacy folder. These tools can be used with blast, diamond, and kraken and visualize directly in krona using iuc's krona wrapper.