From 3ed87bff0a88c43c5f5018023d8f2b8e2fbd0323 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 6 Sep 2022 14:01:14 +0200 Subject: [PATCH 1/5] linter: allow options elements in data params linting of valid childs for params has been added here: https://github.com/galaxyproject/galaxy/pull/12232 options for data params has been forgotten in addition also checks for valid attribs and filter types has been added to linting --- lib/galaxy/tool_util/linters/inputs.py | 19 ++++++++- test/unit/tool_util/test_tool_linters.py | 52 +++++++++++++++++++++++- 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 6a7e602f7c0..3fcf9b5068f 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -108,7 +108,7 @@ PARAMETER_VALIDATOR_TYPE_COMPATIBILITY = { } PARAM_TYPE_CHILD_COMBINATIONS = [ - ("./options", ["select", "drill_down"]), + ("./options", ["data", "select", "drill_down"]), ("./options/option", ["drill_down"]), ("./column", ["data_column"]), ] @@ -167,6 +167,23 @@ def lint_inputs(tool_xml, lint_ctx): lint_ctx.warn( f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", node=param ) + options = param.findall("./options") + if len(options) == 1: + if len(options[0].attrib) > 0: + lint_ctx.error( + f"Data parameter [{param_name}] uses invalid attributes: {options[0].attrib}", node=param + ) + elif len(options) > 1: + lint_ctx.error(f"Data parameter [{param_name}] contains multiple options elements.", node=options[1]) + # for data params only filters with key='build' of type='data_meta' are allowed + filters = param.findall("./options/filter") + for f in filters: + if f.get("key") != "dbkey" or f.get("type") != "data_meta": + lint_ctx.error( + f'Data parameter [{param_name}] for filters only type="data_meta" and key="dbkey" are allowed, found type="{f.get("type")}" and key="{f.get("key")}"', + node=f, + ) + elif param_type == "select": # get dynamic/statically defined options dynamic_options = param.get("dynamic_options", None) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index cbc0a2fc88d..2d28f3b8531 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -190,6 +190,31 @@ INPUTS_DATA_PARAM = """ """ +INPUTS_DATA_PARAM_OPTIONS = """ + + + + + + + + + +""" + +INPUTS_DATA_PARAM_INVALIDOPTIONS = """ + + + + + + + + + + +""" + INPUTS_CONDITIONAL = """ @@ -1012,6 +1037,31 @@ def test_inputs_data_param(lint_ctx): assert not lint_ctx.error_messages +def test_inputs_data_param_options(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_OPTIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.valid_messages + assert "Found 1 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_data_param_invalidoptions(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_INVALIDOPTIONS) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.valid_messages + assert "Found 1 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.warn_messages + assert "Data parameter [valid_name] contains multiple options elements." in lint_ctx.error_messages + assert ( + 'Data parameter [valid_name] for filters only type="data_meta" and key="dbkey" are allowed, found type="expression" and key="None"' + in lint_ctx.error_messages + ) + assert len(lint_ctx.error_messages) == 2 + + def test_inputs_conditional(lint_ctx): tool_source = get_xml_tool_source(INPUTS_CONDITIONAL) run_lint(lint_ctx, inputs.lint_inputs, tool_source) @@ -1197,7 +1247,7 @@ def test_inputs_type_child_combinations(lint_ctx): assert not lint_ctx.valid_messages assert not lint_ctx.warn_messages assert ( - "Parameter [text_param] './options' tags are only allowed for parameters of type ['select', 'drill_down']" + "Parameter [text_param] './options' tags are only allowed for parameters of type ['data', 'select', 'drill_down']" in lint_ctx.error_messages ) assert ( From 6ebfa1ca6620203d75d319d912dd7fe002ca8ec1 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 6 Sep 2022 14:20:56 +0200 Subject: [PATCH 2/5] also improve docs --- lib/galaxy/tool_util/xsd/galaxy.xsd | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 90cc1ccf6dd..603f40d9b0b 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -3968,14 +3968,18 @@ dataset for the contained input of the type specified using the ``type`` tag. `` tag when the ``type`` attribute value is ``select`` or -``data`` and used to dynamically generated lists of options. This tag set -dynamically creates a list of options whose values can be -obtained from a predefined file stored locally or a dataset selected from the -current history. +``data`` and used to dynamically generated lists of options. + +For data parameters it can only be used to restrict inputs to datasets having +the same ``dbkey`` like another input by using a ``data_meta`` filter. See for +instance here: [/tools/maf/interval2maf.xml](https://github.com/galaxyproject/galaxy/blob/master/tools/maf/interval2maf.xml) + +For select parameters this tag set dynamically creates a list of options whose +values can be obtained from a predefined file stored locally or a dataset +selected from the current history. There are at least five basic ways to use this tag - four of these correspond to a ``from_XXX`` attribute on the ``options`` directive and the other is to From 1cb1ac3dfd4649a45071c10882fd7f2fcb6e8597 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 19 Sep 2022 10:24:49 +0200 Subject: [PATCH 3/5] adapt linter to check also for options_filter_attribute --- lib/galaxy/tool_util/linters/inputs.py | 26 +++++++++++++++++------ test/unit/tool_util/test_tool_linters.py | 27 ++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 3fcf9b5068f..16455b51a32 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -168,21 +168,35 @@ def lint_inputs(tool_xml, lint_ctx): f"Param input [{param_name}] with no format specified - 'data' format will be assumed.", node=param ) options = param.findall("./options") + has_options_filter_attribute = False if len(options) == 1: - if len(options[0].attrib) > 0: - lint_ctx.error( - f"Data parameter [{param_name}] uses invalid attributes: {options[0].attrib}", node=param - ) + for oa in options[0].attrib: + if oa == "options_filter_attribute": + has_options_filter_attribute = True + else: + lint_ctx.error(f"Data parameter [{param_name}] uses invalid attribute: {oa}", node=param) elif len(options) > 1: lint_ctx.error(f"Data parameter [{param_name}] contains multiple options elements.", node=options[1]) # for data params only filters with key='build' of type='data_meta' are allowed filters = param.findall("./options/filter") for f in filters: - if f.get("key") != "dbkey" or f.get("type") != "data_meta": + if not f.get("ref"): lint_ctx.error( - f'Data parameter [{param_name}] for filters only type="data_meta" and key="dbkey" are allowed, found type="{f.get("type")}" and key="{f.get("key")}"', + f"Data parameter [{param_name}] filter needs to define a ref attribute", node=f, ) + if has_options_filter_attribute: + if f.get("type") != "data_meta": + lint_ctx.error( + f'Data parameter [{param_name}] for filters only type="data_meta" is allowed, found type="{f.get("type")}"', + node=f, + ) + else: + if f.get("key") != "dbkey" or f.get("type") != "data_meta": + lint_ctx.error( + f'Data parameter [{param_name}] for filters only type="data_meta" and key="dbkey" are allowed, found type="{f.get("type")}" and key="{f.get("key")}"', + node=f, + ) elif param_type == "select": # get dynamic/statically defined options diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 2d28f3b8531..533dcd31e36 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -195,7 +195,19 @@ INPUTS_DATA_PARAM_OPTIONS = """ - + + + + + +""" + +INPUTS_DATA_PARAM_OPTIONS_FILTER_ATTRIBUTE = """ + + + + + @@ -1047,6 +1059,16 @@ def test_inputs_data_param_options(lint_ctx): assert not lint_ctx.error_messages +def test_inputs_data_param_options_filter_attribute(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_OPTIONS_FILTER_ATTRIBUTE) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert not lint_ctx.valid_messages + assert "Found 1 input parameters." in lint_ctx.info_messages + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + def test_inputs_data_param_invalidoptions(lint_ctx): tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_INVALIDOPTIONS) run_lint(lint_ctx, inputs.lint_inputs, tool_source) @@ -1055,11 +1077,12 @@ def test_inputs_data_param_invalidoptions(lint_ctx): assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.warn_messages assert "Data parameter [valid_name] contains multiple options elements." in lint_ctx.error_messages + assert "Data parameter [valid_name] filter needs to define a ref attribute" in lint_ctx.error_messages assert ( 'Data parameter [valid_name] for filters only type="data_meta" and key="dbkey" are allowed, found type="expression" and key="None"' in lint_ctx.error_messages ) - assert len(lint_ctx.error_messages) == 2 + assert len(lint_ctx.error_messages) == 3 def test_inputs_conditional(lint_ctx): From fb60ec257f7e0d62364c8c0d640cb2a5ad290a39 Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 30 Jan 2023 11:26:50 +0100 Subject: [PATCH 4/5] Fix test name typo Co-authored-by: Marius van den Beek --- test/unit/tool_util/test_tool_linters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index 533dcd31e36..ff722d2e091 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -1069,7 +1069,7 @@ def test_inputs_data_param_options_filter_attribute(lint_ctx): assert not lint_ctx.error_messages -def test_inputs_data_param_invalidoptions(lint_ctx): +def test_inputs_data_param_invalid_options(lint_ctx): tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_INVALIDOPTIONS) run_lint(lint_ctx, inputs.lint_inputs, tool_source) assert not lint_ctx.valid_messages From 327e4221895ff67efe4accb5eea98606f594162f Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 30 Jan 2023 12:40:05 +0100 Subject: [PATCH 5/5] Reformulate doc --- lib/galaxy/tool_util/xsd/galaxy.xsd | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 603f40d9b0b..7db7caf5928 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -3973,8 +3973,7 @@ for an example of how to use this tag set. This tag set is optionally contained within the ```` tag when the ``type`` attribute value is ``select`` or ``data`` and used to dynamically generated lists of options. -For data parameters it can only be used to restrict inputs to datasets having -the same ``dbkey`` like another input by using a ``data_meta`` filter. See for +For data parameters this tag can be used to restrict possible input datasets to datasets that match the ``dbkey`` of another data input by including a ``data_meta`` filter. See for instance here: [/tools/maf/interval2maf.xml](https://github.com/galaxyproject/galaxy/blob/master/tools/maf/interval2maf.xml) For select parameters this tag set dynamically creates a list of options whose