Improved CTD to galaxy conversion #54

bernt-matthias · 2019-12-29T18:41:10Z

This is the result of my work for the auto generation of the Galaxy OpenMS wrappers.

adds testing
hardcoded params now as json which allows to do a lot more now (e.g. blacklisting and overwriting of xml/ctd attributes)
major overhaul of most parameters .. see also Add unit tests for CWL #48, disable quoting in command block #52, Lesson learned from unit tests #53

superseeds #50 and #46

bernt-matthias · 2019-12-29T18:41:36Z

ping @Tomnl @bgruening

bgruening · 2019-12-29T18:48:03Z

@bernt-matthias can you run this and create a PR against galaxyp/openms ...?

Thanks a lot, this is awesome!

bernt-matthias · 2019-12-29T18:52:52Z

This is comming soon. Still fighting with a few failing tests (yeah we have tests :) ).

this will also soon be ready galaxyproject/galaxy#8109 .. maybe we should check if we can have this in 20.01

bgruening · 2019-12-29T18:55:05Z

Jupp, I think retargeting to 20.01 could work. That's a cool present! Thanks so much.

Tomnl · 2020-01-09T08:41:41Z

This is great! Thanks @bernt-matthias !

ping @RJMW

RJMW · 2020-01-09T09:16:40Z

Wow! - Thanks for the all the efforts @bernt-matthias

- removed test file from OpenMS - fixed xsd link - extended select, i/o file test data

OpenMS (ctd 1.6.2) uses string with restriction true/false for booleans - those with default false are flags, ie `-param` - otherwise `-param false` needs to be given for simplification the neccessary command line is now stored directly in the true/falsevalue attributes also added more docs

selects: - add possibility of selects with multiple=true from ITEMLIST - just add single quotes around all option values optional: - optional was only set for params with restrictions now its always set (one might remove it for bools...) also add more documentation

dont set it via the param's value attribute, but the selected attribute of the options (the latter was already the case)

- add quotes for multiple,optional file inputs - remove the (now deprecated) size attribute of text fields - moved parameter to argument attribute - allow for empty defaults in int/float inputs

also restored quoting in ifile and repeats

Previously itemlists wo restrictions were rendered as repeats. Problem is that repeats do not allow to enter default values. Also a select is no option since the user can't add values So I suggest to use a text field allowing for comma separated list of strings, integers, or floats. The correctness of the input is verified by by validators and sanitizers. I'm optimistic that listitems do not contain ',' (which would be a problem), since restrictions of restricted itemslists are given as comma separated list.

also added itemlists with default to ctd

mr-c · 2021-02-08T10:34:50Z

Hello @bernt-matthias !

I'm working on the CWL export, and I see that you've made a lot of fixes that I have, or that I need. Do you anticipate being able to merge soon?

Or shall we peel off some of these fixes into other "bite-sized" PRs and merge them individually?

bernt-matthias · 2021-02-08T10:41:39Z

Hi @mr-c .. good point. I would be happy about comments and to get this merged.

I would say that this is functional. We used this branch for the release of OpenMS Galaxy tools for version 2.5 and 2.6.

But:

update of the docs would be nice. in particular parameter hardcoding etc.
I guess also the failing tests should be considered

mr-c · 2021-02-08T10:50:30Z

.travis.yml

+
+#   - virtualenv .venv
+#   - . .venv/bin/activate
+  - pip install git+https://github.com/bernt-matthias/CTDopts@topic/no-1-2x


WorkflowConversion:master is up to date with all commits from bernt-matthias:topic/no-1-2x.

But the reverse is not true, and is the source of at least one of the failing tests

bernt-matthias/CTDopts@topic/no-1-2x...WorkflowConversion:master#diff-a55c430e446e6696bb3f110ad5ce8607e83273a276ce6aaef8816a285b014503R50

mr-c · 2021-02-08T10:51:22Z

.travis.yml

+# TODO fail expected due to 
+# https://github.com/galaxyproject/galaxy/pull/9081 which will be in 20.01 and 
+# empty.xml (no inputs) could be removed and  


Is this still the case? The mentioned PR was merged last year

and enable planemo lint

mr-c · 2021-02-08T12:05:32Z

FAIL: test_galaxy_cli_bool_ctd (tests.test_galaxy_cli.GalaxyCliTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/WorkflowConversion/CTDConverter/tests/test_galaxy_cli.py", line 51, in test_galaxy_cli_bool_ctd
    self._compare_cli_output('bool')
  File "/home/travis/build/WorkflowConversion/CTDConverter/tests/test_galaxy_cli.py", line 48, in _compare_cli_output
    self.assertEqual(new_l[i].rstrip(), old_l[i].rstrip())
AssertionError: '#import os.path' != ''
- #import os.path
+

mr-c · 2021-02-09T08:12:59Z

PyTests pass!

Looks like some of the planemo-run tests fail. Shall we skip them and merge?

ofile-mult (Test #1): failed
ofile-mult-typeparam (Test #1): failed

bernt-matthias · 2021-02-09T10:36:00Z

I will fix this this afternoon / tomorrow .. just need to remove a few test cases which are unreasonable (I think).

if the format of an output is determined by a select then set the select-default as format of the output and add `<change_format><when>` only for the other choices

mr-c · 2021-02-10T11:16:37Z

so close, back to the error seen in #54 (comment)

bernt-matthias · 2021-02-10T12:21:15Z

Just discussing with OpenMS developers that a change analogous to 4912610 might also be necessary for outputs :(

bernt-matthias · 2021-02-10T12:28:02Z

Wait, this is already happening :) .. But I was thinking about to change this slightly.

Now for list input / outputs we create a folder PARAMETER_NAME/GALAXY_UNIQUE_DATASET_ID/ and place the symlink with the name label.ext in there.

I'm thinking about using an integers 0,1,2... instead of GALAXY_UNIQUE_DATASET_ID would be a bit nicer.

And should be easy to implement.

bernt-matthias · 2021-02-10T12:45:12Z

Hah. Tests are passing. Give me a bit for the last step...

simpler and nicer

bernt-matthias · 2021-02-10T13:59:53Z

OK. If tests here and there galaxyproteomics/tools-galaxyp#555 are passing its ready.

bernt-matthias · 2021-02-10T17:00:45Z

Looks good now. Last failing OpenMS tool is fixed. You may merge if you like.

For another automatic tool generation project (https://github.com/hexylena/argparse2tool/) I would like to reuse the ParameterHardcoder, i.e.

CTDConverter/common/utils.py

Line 27 in 9247ce5

class ParameterHardcoder:

and

CTDConverter/common/utils.py

Line 215 in 9247ce5

def parse_hardcoded_parameters(hardcoded_parameters_file):

Do you think its worth to factor this out into a separate project or should I just import CTDconverter?

Btw. argparse2tool can also generate CWL tools.

mr-c · 2021-02-10T17:04:58Z

Looks good now. Last failing OpenMS tool is fixed. You may merge if you like.

Huzzah! I don't have write access to this repository yet, otherwise I would merge it :-)

Do you think its worth to factor this out into a separate project or should I just import CTDconverter?

This isn't a javascript project, so I'd say that isn't enough for a separate package :-P

test data: initial commit

896c380

bernt-matthias mentioned this pull request Jan 14, 2020

OpenMS 2.5 .. autogenerated galaxyproteomics/tools-galaxyp#415

Merged

29 tasks

Tomnl and others added 22 commits January 27, 2020 19:40

add unit tests

c549147

switch CTDopts branch

2a4ae8d

fixed CTDopts branch name

81f5465

improved indentation of float.ctd

1ebc562

update test results

79e516d

added test files for bool, in/out files

8d58f90

- added filetypes to test call

c8ec983

- removed test file from OpenMS - fixed xsd link - extended select, i/o file test data

added filetypes.txt

a2efdb1

advanced repeats were not advanced

c3947af

add missing tests

1376b84

make help and command CDATA

616074d

fix specification of select default value

84cd70f

dont set it via the param's value attribute, but the selected attribute of the options (the latter was already the case)

fix indentation of advanced parameters in command block

c966a9d

use argument attribute, remove size attribute

e33b637

- add quotes for multiple,optional file inputs - remove the (now deprecated) size attribute of text fields - moved parameter to argument attribute - allow for empty defaults in int/float inputs

int and float wo default need empty value attribute

828f6ba

also restored quoting in ifile and repeats

simplify code for advanced node generation

0dfe02e

remove duplicate format entries in inputs

c7d3905

remove sanitizer from input files

0c23fd0

also added itemlists with default to ctd

infile input needs no help info that a comma separated list is needed

e40979a

bernt-matthias force-pushed the topic/cdata branch from d8cea18 to 4912610 Compare February 7, 2021 19:12

mr-c reviewed Feb 8, 2021

View reviewed changes

use latest CTDOpts for testing

fa54b48

and enable planemo lint

bernt-matthias force-pushed the topic/cdata branch from 0e58955 to fa54b48 Compare February 8, 2021 11:23

fix test case

0446544

bernt-matthias added 2 commits February 8, 2021 15:08

update test data

588b50f

few testing fixes

b7a40b7

bernt-matthias added 2 commits February 10, 2021 11:14

change format only for non defaults

b044b08

if the format of an output is determined by a select then set the select-default as format of the output and add `<change_format><when>` only for the other choices

exclude some unsupported/nonsense test cases

35e5c54

bernt-matthias force-pushed the topic/cdata branch from 850eea7 to 35e5c54 Compare February 10, 2021 10:15

bernt-matthias added 3 commits February 10, 2021 12:35

import os

67d36e0

update docs

2ed0959

flake8 fixes

966007f

bernt-matthias force-pushed the topic/cdata branch from 1e1a353 to 966007f Compare February 10, 2021 12:14

use continuous integers instead of dataset id

9247ce5

simpler and nicer

jpfeuffer merged commit 84c5867 into WorkflowConversion:master Feb 10, 2021

mr-c mentioned this pull request Mar 15, 2021

Order Galaxy XML to IUC recommendations #46

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved CTD to galaxy conversion #54

Improved CTD to galaxy conversion #54

bernt-matthias commented Dec 29, 2019

bernt-matthias commented Dec 29, 2019

bgruening commented Dec 29, 2019

bernt-matthias commented Dec 29, 2019

bgruening commented Dec 29, 2019

Tomnl commented Jan 9, 2020

RJMW commented Jan 9, 2020

mr-c commented Feb 8, 2021

bernt-matthias commented Feb 8, 2021

mr-c Feb 8, 2021

mr-c Feb 8, 2021

mr-c commented Feb 8, 2021

mr-c commented Feb 9, 2021

bernt-matthias commented Feb 9, 2021

mr-c commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

mr-c commented Feb 10, 2021

Improved CTD to galaxy conversion #54

Improved CTD to galaxy conversion #54

Conversation

bernt-matthias commented Dec 29, 2019

bernt-matthias commented Dec 29, 2019

bgruening commented Dec 29, 2019

bernt-matthias commented Dec 29, 2019

bgruening commented Dec 29, 2019

Tomnl commented Jan 9, 2020

RJMW commented Jan 9, 2020

mr-c commented Feb 8, 2021

bernt-matthias commented Feb 8, 2021

mr-c Feb 8, 2021

Choose a reason for hiding this comment

mr-c Feb 8, 2021

Choose a reason for hiding this comment

mr-c commented Feb 8, 2021

mr-c commented Feb 9, 2021

bernt-matthias commented Feb 9, 2021

mr-c commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

bernt-matthias commented Feb 10, 2021

mr-c commented Feb 10, 2021