-
Notifications
You must be signed in to change notification settings - Fork 70
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 genomes on server functionality #164
Conversation
Can you point me at the Galaxy tool author documentation for this new (to me) functionality? I'm wondering e.g. if it requires a specific version of Galaxy (and if it would fail gracefully on an older one). |
@peterjc https://docs.galaxyproject.org/en/latest/dev/schema.html#tool-inputs-param-options |
I had problems in handling the term "value_label" in the output name, because i didn't understand it quite well. Is it only for having "Protein" instead of "prot" in the name of the output? Maybe an additional test for the genome on server functionality would be nice? |
Would the current nested conditionals break old workflows etc? Might therefore this be a safer approach: <param argument="-dbtype" type="select" display="radio" label="Molecule type of input">
<option value="prot">protein from your history</option>
<option value="nucl">nucleotide from your history</option>
<option value="nucl-server">nucleotide from server</option>
</param> |
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.
Nice :) Thanks for adding the new test!
You will need to bump the @VERSION_SUFFIX@ eventually.
@peterjc in a sense, yes. Though of course only for people that are trying to run their workflow against a server that doesn't have the old tool version (requested by the workflow) installed. We normally consider this an acceptable drawback though I don't like making the life of users unnecessarily hard either. After testing a few alternatives I suggested to @elischberg to use the repeat elements (instead of the original multi-selects) because that lets you combine FASTAs from the history and server-provided ones when building the DB. |
btw, the use case behind the new feature is a viral primer scheme design tool that lets you exclude primer candidates based on them having matches in a custom BLAST DB, e.g. to avoid amplification of host DNA. With the new option, users will be able to create their desired subtraction DB as long as Galaxy Europe has the corresponding host reference installed. |
Given both @elischberg and @wm75 have been testing this (I have not), I'm happy in principle to merge this. There are not any other imminent pull requests pending, so I suggest you bump the version and describe the changes in https://github.com/peterjc/galaxy_blast/blob/master/tools/ncbi_blast_plus/README.rst - something like:
|
Test failures in GitHub actions:
Looking at the HTML file
Happily that looks like an easy fix [already suggested by @wm75, which I have just committed. Hopefully won't complicate further updates to the pull request - ask if you need help with git] |
Spotted by @wm75 during review Co-authored-by: Wolfgang Maier <[email protected]>
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.
Thanks @peterjc and @elischberg - this looks good to me then!
Squash-and-merged, thank you both. In theory this will auto-deploy to the Tool Shed... https://github.com/peterjc/galaxy_blast/actions/runs/8003981806 |
Test Tool Shed failed - could be in flux? Main Tool Shed also failed:
Looks like we need to add |
Huh - the automated deployment did go though even without the Anyway, these changes are now live on the Tool Shed 🎉 |
The Test TS has been down for the last 2 months. |
@nsoranzo thanks - I didn't double check the TTS status 👍 |
... meaning the new version should be available on Galaxy Europe by Monday. Thanks a lot! |
No description provided.