-
Notifications
You must be signed in to change notification settings - Fork 58
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 enzyme list to morpheus tool #766
add enzyme list to morpheus tool #766
Conversation
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 @mthang, 2 comments inline.
tools/morpheus/morpheus.xml
Outdated
@@ -1,49 +1,11 @@ | |||
<tool id="morpheus" name="Morpheus" version="2.255.0"> | |||
<tool id="morpheusid" name="Morpheus" version="@TOOL_VERSION@+galaxy@VERSION_SUFFIX@" profile="@PROFILE@"> |
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.
please don't change the tool ID, otherwise we loose the tool lineage
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 bad. I forgot to change it back after testing.
tools/morpheus/macros.xml
Outdated
@@ -0,0 +1,77 @@ | |||
<macros> | |||
<token name="@TOOL_VERSION@">2.255.0</token> |
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.
the version needs to be bumped
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.
agreed! The latest version is 288. do you want me to bump it up to 288?
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.
Thats a good idea.
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.
@bgruening I bumped the version up to 288 from 255 and updated the output of the test section. Can you check again.
Please also see the failing lint:
|
@@ -150,7 +112,10 @@ | |||
#if str($vm) != 'None': | |||
#set $vmods = str($vm).replace(',',';') | |||
-vm="$vmods" | |||
#end if | |||
#end if | |||
#if str($p) != 'None': |
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.
It can never be None, because the select param is not optional.
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.
Looks good to me! Thanks @mthang
FOR CONTRIBUTOR:
[x]I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
[x]License permits unrestricted use (educational + commercial)
[x]This PR adds a new tool or tool collection
[x]This PR updates an existing tool or tool collection
[]This PR does something else (explain below)
Background:
The morpheus wrapper does not have the parameter (p) which allows user to select enzyme to be used in the morpheus search process. The option p is listed in https://cwenger.github.io/Morpheus/args.html .
In addition to include the p option in the wrapper. I have tidied up the morpheus wrapper by creating a macro file for tool version , tokens, values and enzyme list for the paramters used in the wrapper.