-
Notifications
You must be signed in to change notification settings - Fork 481
Constava #7328
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
Constava #7328
Conversation
Add constava
|
Tool tests are failing on the exact same step as #7243 (See GH action run) so I suspect there is an issue with the CI/CD of the repository perhaps? |
|
Indeed @B0r1sD, in my local workstation the tests are all green with the command provided by the contributing guidelines as well as the planemo lint: Lint: planemo lint ./tools/constava/constava.xml
...
.. CHECK (TestsNoValid): 12 test(s) found.
.. INFO (OutputsNumber): 1 outputs found.
.. INFO (InputsNum): Found 17 input parameters.
.. CHECK (HelpPresent): Tool contains help section.
.. CHECK (HelpValidRST): Help contains valid reStructuredText.
.. CHECK (ToolIDValid): Tool defines an id [constava].
.. CHECK (ToolNameValid): Tool defines a name [Constava].
.. CHECK (ToolProfileValid): Tool specifies profile version [24.2].
.. CHECK (ToolVersionValid): Tool defines a version [1.1.0+galaxy0].
.. INFO (CommandInfo): Tool contains a command.
.. CHECK (CitationsFound): Found 2 citations. Tests: planemo test --install_galaxy tools/constava/constava.xml
...
All 12 test(s) executed passed.
constava (Test #1): passed
constava (Test #2): passed
constava (Test #3): passed
constava (Test #4): passed
constava (Test #5): passed
constava (Test #6): passed
constava (Test #7): passed
constava (Test #8): passed
constava (Test #9): passed
constava (Test #10): passed
constava (Test #11): passed
constava (Test #12): passedIs there anything that we could change from our side to fix the CI/CD pipeline @bgruening ? Thanks in advance |
|
Please test with |
|
Thanks @SaimMomin12 for the info ! Let us know if the PR needs more changes. We're looking forward to seeing Constava on Galaxy soon 🚀 Kind regards from Brussels to you all |
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 many tests, nice! A few more nitpicking comments - sorry.
tools/constava/constava.xml
Outdated
| <environment_variable name="PYTHON_TQDM_DISABLE">1</environment_variable> | ||
| </environment_variables> | ||
| <inputs> | ||
| <section name="input_options" title="Input Options" expanded="true" help="As input data the backbone dihedral angles extracted from the conformational ensemble need to be provided. Important: Given Constava extracts RESNAME and RESINDEX from filenames when using XVG format, your files must follow this regex 'ramaPhiPsi([A-Z][A-Z0-9][A-Z0-9])([0-9]+).xvg'"> |
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.
should this help section be really at the section level?
It would fit better at the format=data level I think?
Please also consider to rephrase it. Its not a filename in the Galaxy context, but rather the dataset name, that a user can change :(
tools/constava/constava.xml
Outdated
| <option value="bootstrap">Bootstrap sampling</option> | ||
| </param> | ||
| <when value="window"> | ||
| <param name="window_size" type="text" label="Window size (space-separated integers)" value="3" help="Specify window sizes for moving frame analysis, e.g., '3 5 7'. Each reading frame consists of consecutive samples. Multiple values can be provided."/> |
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.
you could add here a "validator" that checks that the input is really just numbers and spaces
tools/constava/constava.xml
Outdated
| <param name="window_size" type="text" label="Window size (space-separated integers)" value="3" help="Specify window sizes for moving frame analysis, e.g., '3 5 7'. Each reading frame consists of consecutive samples. Multiple values can be provided."/> | ||
| </when> | ||
| <when value="bootstrap"> | ||
| <param name="bootstrap_size" type="text" label="Bootstrap size (space-separated integers)" value="3" help="Do inference using N samples obtained through bootstrapping. Specify bootstrap sizes, e.g., '10 20 30'. Samples obtained through bootstrapping. Multiple values can be provided."/> |
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.
some here a validator would be nice
Co-authored-by: Björn Grüning <[email protected]>
All are useful comments, thank you for your patience reviewing the PR @bgruening and @SaimMomin12. CI/CD passed green, would you mind if I asked for a new review round ? |
|
Let's ship it :) If you feel like it: https://galaxyproject.eu/posts/2020/08/22/three-steps-to-galaxify-your-tool/ |
|
Thank you so much reviewers for your patience and guidance!!! I'll open the PR in |
|
Here it is: usegalaxy-eu/usegalaxy-eu-tools#921 |
|
Thanks a lot @agdiaz. Great work! |


FOR CONTRIBUTOR:
Original PR that contains all information about the tool by @agdiaz: #6235. As that repo was archived, making a new PR was necessary.
@agdiaz feel free to add any additional information, in addition to your original, elaborative PR.