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

WIP: enhance shed_tools to install from native data_manager files #93

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bgruening
Copy link
Member

No description provided.

Copy link
Contributor

@rhpvorderman rhpvorderman left a comment

Choose a reason for hiding this comment

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

I don't know if this will work. The way of parsing stuff will not work with our current test file

repositories = list()
dms = yaml.load(open(options.data_manager_yaml))
for dm in dms['data_managers']:
tokens = dm['id'].split('/')
Copy link
Contributor

Choose a reason for hiding this comment

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

In a data managers file you can also just give the tool ID so then this does not work, and a lot of errors will follow...
There should be some error handling here. Also tests should be added to include a parsable file and a non-parsable (but still valid) file.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can/Should we enforce long-tool-ids for reproducibility reasons?

Copy link
Contributor

Choose a reason for hiding this comment

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

The data manager job is saved in galaxy's history. So that is not necessary IMO.Also, long tool ids are quite inconvenient. Are there any people who have multiple fasta indexes of the same genome on their galaxy just because of the version of their indexer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh its not about reproducibility in one Galaxy instance, but across instances... https://github.com/bgruening/idc

Are there any people who have multiple fasta indexes of the same genome on their galaxy just because of the version of their indexer?

Not sure, but they are different data managers where the version is crucial. And keep in mind that data managers can be way more than genome indices.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are valid reasons to support long tool IDs but not necessarily to enforce them.
Would it be possible to put in an if statement here that recognizes a long tool ID from a short tool ID and else throws a proper exception? For the long tool ID the following code looks good. For a short tool ID some code could be added that fetches the repo owner from toolshed, preferring iuc and devteam if there are multiple owners and selecting the first one in alphabetic order otherwise whilst throwing a warning that there were multiple owners.

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