From e66f4bb70bd3d977e5f1cd6758ecb2ae3da7c6a2 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 17 Nov 2021 21:14:17 +0100 Subject: [PATCH 01/30] add data table filter --- lib/galaxy/tool_util/xsd/galaxy.xsd | 16 +++++- .../tools/parameters/dynamic_options.py | 56 +++++++++++++++++++ test/functional/tool-data/test_file.tsv | 2 + test/functional/tools/filter_data_table.xml | 45 +++++++++++++++ test/functional/tools/sample_tool_conf.xml | 1 + 5 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 test/functional/tool-data/test_file.tsv create mode 100644 test/functional/tools/filter_data_table.xml diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index ce54c99c2d67..7b9c033c996c 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -4803,8 +4803,9 @@ Currently the following filters are defined: * ``data_meta`` populate or filter options based on the metadata of another input parameter specified by ``ref``. If a ``column`` is given options are filtered for which the entry in this column ``column`` is equal to metadata of the input parameter specified by ``ref``. If no ``column`` is given the metadata value of the referenced input is added to the options list (in this case the corresponding ``options`` tag must not have the ``from_data_table`` or ``from_dataset`` attributes). In both cases the desired metadata is selected by ``key``. +* ``data_table`` remove values according to the entries of a data table. Remove options where the value in ``column`` appears in the data table ``table_name`` in column ``table_column``. Setting ``keep`` will to ``true`` will keep only entries also appearing in the data table. -The ``static_value`` and ``regexp`` filters can be inverted by setting ``keep`` to true. +The ``static_value``, ``regexp``, and ``data_table`` filters can be inverted by setting ``keep`` to true. * ``add_value``: add an option with a given ``name`` and ``value`` to the options. By default, the new option is appended, with ``index`` the insertion position can be specified. * ``remove_value``: remove a value from the options. Either specified explicitly with ``value``, the value of another input specified with ``ref``, or the metadata ``key`` of another input ``meta_ref``. @@ -4978,7 +4979,7 @@ only used if ``multiple`` is set to ``true``.]]> If ``true``, keep columns matching the value, if ``false`` discard columns matching the value. Used when ``type`` is -either ``static_value``, ``regexp`` or ``param_value``. +either ``static_value``, ``regexp``, ``param_value`` or ``data_table``. @@ -5025,6 +5026,16 @@ from the list. Only used if ``type`` is ``attribute_value_splitter``. This is used to separate attributes and values from each other within an attribute-value pair, i.e. ``=`` if the target content is ``A=V; B=W; C=Y``. Defaults to whitespace. + + + Only used when ``type`` is +``data_table``. The name of the data table to use. + + + + + Only used when ``type`` is +``data_table``. The column of the data table to use (0 based index or column name). @@ -7091,6 +7102,7 @@ and ``bibtex`` are the only supported options. + diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 03a35c249fc5..5ffac16b4cfd 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -517,6 +517,59 @@ def filter_options(self, options, trans, other_values): return sorted(options, key=lambda x: x[self.column], reverse=self.reverse) +class DataTableFilter(Filter): + """ + Filters a list of options by entries present in a data table, i.e. + option[column] needs to be in the specified data table column + + Type: data_table + + Required Attributes: + + - column: column in options to compare with + - table_name: data table to use + - data_table_column: data table column to use + + Optional Attributes: + + - keep: Keep options where option[column] is in the data table column (True) + Discard columns matching value (False) + + """ + def __init__(self, d_option, elem): + Filter.__init__(self, d_option, elem) + self.table_name = elem.get("table_name", None) + assert self.table_name is not None, "Required 'table_name' attribute missing from filter" + column = elem.get("column", None) + assert column is not None, "Required 'column' attribute missing from filter" + self.column = d_option.column_spec_to_index(column) + self.data_table_column = elem.get("data_table_column", None) + assert self.data_table_column is not None, "Required 'data_table_column' attribute missing from filter" + self.keep = string_as_bool(elem.get("keep", 'True')) + + def filter_options(self, options, trans, other_values): + # get column from data table, by index or column name + entries = None + try: + entries = set([f[int(self.data_table_column)] for f in trans.app.tool_data_tables[self.table_name].get_fields()]) + except TypeError: + pass + try: + entries = set([f[self.data_table_column] for f in trans.app.tool_data_tables[self.table_name].get_named_fields_list()]) + except KeyError: + pass + if entries is None: + log.error(f"could not get data from column {self.data_table_column} from data_table {self.table_name}") + return options + + rval = [] + for o in options: + log.error(f"o {o}") + if self.keep == (o[self.column] in entries): + rval.append(o) + return rval + + filter_types = dict( data_meta=DataMetaFilter, param_value=ParamValueFilter, @@ -528,6 +581,7 @@ def filter_options(self, options, trans, other_values): add_value=AdditionalValueFilter, remove_value=RemoveValueFilter, sort_by=SortByColumnFilter, + data_table=DataTableFilter, ) @@ -580,10 +634,12 @@ def load_from_parameter(from_parameter, transform_lines=None): data_file = data_file.strip() if not os.path.isabs(data_file): full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) + log.error(f"full_path {full_path}") if os.path.exists(full_path): self.index_file = data_file with open(full_path) as fh: self.file_fields = self.parse_file_fields(fh) + log.error(f"exists file_fields {self.file_fields}") else: self.missing_index_file = data_file elif dataset_file is not None: diff --git a/test/functional/tool-data/test_file.tsv b/test/functional/tool-data/test_file.tsv new file mode 100644 index 000000000000..22722e11bbae --- /dev/null +++ b/test/functional/tool-data/test_file.tsv @@ -0,0 +1,2 @@ +hg19_value hg19 hg19_name hg19_path +absent_value absent absent_name absent_path diff --git a/test/functional/tools/filter_data_table.xml b/test/functional/tools/filter_data_table.xml new file mode 100644 index 000000000000..6b7709a3fc69 --- /dev/null +++ b/test/functional/tools/filter_data_table.xml @@ -0,0 +1,45 @@ + + Filter on datatable entries + '$output' + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/sample_tool_conf.xml b/test/functional/tools/sample_tool_conf.xml index 9aa784ae80f0..e9413b6fc725 100644 --- a/test/functional/tools/sample_tool_conf.xml +++ b/test/functional/tools/sample_tool_conf.xml @@ -61,6 +61,7 @@ + From 26043cd29c48befb2bedabc2acf9a93cc36c7d0f Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 18 Nov 2021 15:46:23 +0100 Subject: [PATCH 02/30] add more linting for filters --- lib/galaxy/tool_util/linters/inputs.py | 58 +++++++++++++++++++- test/unit/tool_util/test_tool_linters.py | 67 ++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 4721d5806447..7103ddd888a6 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -19,8 +19,34 @@ "add_value", "remove_value", "sort_by", + "data_table" ] +FILTER_REQUIRED_ATTRIBUTES = { + "data_meta": ["type", "ref", "key"], # column needs special treatment + "param_value": ["type", "ref", "column"], + "static_value": ["type", "column", "value"], + "regexp": ["type", "column", "value"], + "unique_value": ["type", "column"], + "multiple_splitter": ["type", "column"], + "attribute_value_splitter": ["type", "column"], + "add_value": ["type", "value"], + "remove_value": ["type"], # this is handled separately + "sort_by": ["type", "column"], + "data_table": ["type", "column", "table_name", "data_table_column"], +} + +FILTER_ALLOWED_ATTRIBUTES = deepcopy(FILTER_REQUIRED_ATTRIBUTES) +FILTER_ALLOWED_ATTRIBUTES["static_value"].append("keep") +FILTER_ALLOWED_ATTRIBUTES["regexp"].append("keep") +FILTER_ALLOWED_ATTRIBUTES["data_meta"].extend(["column", "multiple", "separator"]) +FILTER_ALLOWED_ATTRIBUTES["param_value"].extend(["keep", "ref_attribute"]) +FILTER_ALLOWED_ATTRIBUTES["multiple_splitter"].append("separator") +FILTER_ALLOWED_ATTRIBUTES["attribute_value_splitter"].extend(["pair_separator", "name_val_separator"]) +FILTER_ALLOWED_ATTRIBUTES["add_value"].extend(["name", "index"]) +FILTER_ALLOWED_ATTRIBUTES["remove_value"].extend(["value", "ref", "meta_ref", "key"]) +FILTER_ALLOWED_ATTRIBUTES["data_table"].append("keep") + ATTRIB_VALIDATOR_COMPATIBILITY = { "check": ["metadata"], "expression": ["substitute_value_in_message"], @@ -125,6 +151,13 @@ def lint_inputs(tool_xml, lint_ctx): if tool_node is None: tool_node = tool_xml.getroot() num_inputs = 0 + + # get the set of param names + param_names = set() + for param in inputs: + if "name" in param.attrib or "argument" in param.attrib: + param_names.add(_parse_name(param.attrib.get("name"), param.attrib.get("argument"))) + for param in inputs: num_inputs += 1 param_attrib = param.attrib @@ -261,7 +294,30 @@ def lint_inputs(tool_xml, lint_ctx): continue if ftype in ["add_value", "data_meta"]: filter_adds_options = True - # TODO more linting of filters + # check for required attributes for filter (remove_value needs a bit more logic here) + for attrib in FILTER_REQUIRED_ATTRIBUTES[ftype]: + if attrib not in f.attrib: + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}' filter misses required attribute '{attrib}'") + if ftype == "remove_value": + if not (("value" in f.attrib and "ref" not in f.attrib and "meta_ref" not in f.attrib and "key" not in f.attrib) + or ("value" not in f.attrib and "ref" in f.attrib and "meta_ref" not in f.attrib and "key" not in f.attrib) + or ("value" not in f.attrib and "ref" not in f.attrib and "meta_ref" in f.attrib and "key" in f.attrib)): + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)") + # check for allowed filter attributes (only warning because others are ignored) + for attrib in f.attrib: + if attrib not in FILTER_ALLOWED_ATTRIBUTES[ftype]: + lint_ctx.warn(f"Select parameter [{param_name}] '{ftype}' filter specifies unnecessary attribute '{attrib}'") + # check for references to other inputs + # TODO: currently ref and metaref seem only to work for top level params, + # once this is fixed the linter needs to be extended, e.g. `f.attrib[ref_attrib].split('|')[-1]` + for ref_attrib in ["meta_ref", "ref"]: + if ref_attrib in f.attrib and f.attrib[ref_attrib] not in param_names: + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter attribute '{ref_attrib}' refers to non existing parameter '{f.attrib[ref_attrib]}'") + if ftype == "regexp" and "value" in f.attrib: + try: + re.compile(f.attrib["value"]) + except re.error as re_error: + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter 'value' is not a valid regular expression ({re_error})'") from_file = options[0].get("from_file", None) from_parameter = options[0].get("from_parameter", None) diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index e27a59536e7f..26b24f9359fb 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -485,6 +485,48 @@ """ +INPUTS_FILTER_INCORRECT = """ + + + + + + + + + + + + + + + +""" + +INPUTS_FILTER_CORRECT = """ + + + + + + + + + + + + + + + + + + + + + + +""" # test tool xml for outputs linter OUTPUTS_MISSING = """ @@ -1384,6 +1426,31 @@ def test_inputs_duplicate_names(lint_ctx): assert len(lint_ctx.error_messages) == 2 +def test_inputs_filter_correct(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_FILTER_CORRECT) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert not lint_ctx.warn_messages + assert not lint_ctx.error_messages + + +def test_inputs_filter_correct(lint_ctx): + tool_source = get_xml_tool_source(INPUTS_FILTER_INCORRECT) + run_lint(lint_ctx, inputs.lint_inputs, tool_source) + assert len(lint_ctx.info_messages) == 1 + assert not lint_ctx.valid_messages + assert "Select parameter [select_param] 'regexp' filter specifies unnecessary attribute 'multiple'" in lint_ctx.warn_messages + assert len(lint_ctx.warn_messages) == 1 + assert "Select parameter [select_param] contains filter without type." in lint_ctx.error_messages + assert "Select parameter [select_param] contains filter with unknown type 'typo'." in lint_ctx.error_messages + assert "Select parameter [select_param] 'static_value' filter misses required attribute 'column'" in lint_ctx.error_messages + assert "Select parameter [select_param] 'remove_value'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)" in lint_ctx.error_messages + assert "Select parameter [select_param] 'data_meta'' filter attribute 'ref' refers to non existing parameter 'input'" in lint_ctx.error_messages + assert "Select parameter [select_param] 'regexp'' filter 'value' is not a valid regular expression (nothing to repeat at position 1)'" in lint_ctx.error_messages + assert len(lint_ctx.error_messages) == 6 + + def test_inputs_repeats(lint_ctx): tool_source = get_xml_tool_source(REPEATS) run_lint(lint_ctx, inputs.lint_repeats, tool_source) From 2470e460f33dc8e8f6a3454bfb7ef5e0acd854b7 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 18 Nov 2021 17:16:33 +0100 Subject: [PATCH 03/30] use test/functional/tool-data/ as tool-data dir in tests --- lib/galaxy_test/driver/driver_util.py | 2 +- test/functional/tools/filter_data_table.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy_test/driver/driver_util.py b/lib/galaxy_test/driver/driver_util.py index 4a638f0c9fb8..a7d9e6530820 100644 --- a/lib/galaxy_test/driver/driver_util.py +++ b/lib/galaxy_test/driver/driver_util.py @@ -194,7 +194,6 @@ def setup_galaxy_config( data_manager_config_file = _resolve_relative_config_paths(data_manager_config_file) tool_config_file = _resolve_relative_config_paths(tool_conf) tool_data_table_config_path = _resolve_relative_config_paths(tool_data_table_config_path) - config = dict( admin_users="test@bx.psu.edu", allow_library_path_paste=True, @@ -226,6 +225,7 @@ def setup_galaxy_config( running_functional_tests=True, template_cache_path=template_cache_path, tool_config_file=tool_config_file, + tool_data_path="test/functional/tool-data/", tool_data_table_config_path=tool_data_table_config_path, tool_parse_help=False, tool_path=tool_path, diff --git a/test/functional/tools/filter_data_table.xml b/test/functional/tools/filter_data_table.xml index 6b7709a3fc69..c6e67b27b82b 100644 --- a/test/functional/tools/filter_data_table.xml +++ b/test/functional/tools/filter_data_table.xml @@ -6,7 +6,7 @@ - + From d7c99a0ebe133c81255466dbd7b55d7f3115d54c Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 18 Nov 2021 17:27:45 +0100 Subject: [PATCH 04/30] forbid from_file to reference files outside of tool-data --- lib/galaxy/tools/parameters/dynamic_options.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 5ffac16b4cfd..6fd511b15549 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -632,9 +632,9 @@ def load_from_parameter(from_parameter, transform_lines=None): self.parse_column_definitions(elem) if data_file is not None: data_file = data_file.strip() - if not os.path.isabs(data_file): - full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) - log.error(f"full_path {full_path}") + full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) + full_path = os.path.normpath(full_path) + if safe_contains(self.tool_param.tool.app.config.tool_data_path, full_path) if os.path.exists(full_path): self.index_file = data_file with open(full_path) as fh: @@ -642,6 +642,9 @@ def load_from_parameter(from_parameter, transform_lines=None): log.error(f"exists file_fields {self.file_fields}") else: self.missing_index_file = data_file + else: + log.error(f"'from_file' ({data_file}) references path outside of Galaxy's tool-data dir!") + self.missing_index_file = data_file elif dataset_file is not None: self.meta_file_key = elem.get("meta_file_key", None) self.dataset_ref_name = dataset_file From 64831e00d2bce4359f93235c6f7c614513387659 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 18 Nov 2021 17:55:11 +0100 Subject: [PATCH 05/30] fix linter and syntax errors --- lib/galaxy/tool_util/linters/inputs.py | 3 ++- lib/galaxy/tools/parameters/dynamic_options.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 7103ddd888a6..90a6f1c1506f 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -1,5 +1,6 @@ """This module contains a linting functions for tool inputs.""" import re +from copy import deepcopy from galaxy.util import string_as_bool from ._util import ( @@ -152,7 +153,7 @@ def lint_inputs(tool_xml, lint_ctx): tool_node = tool_xml.getroot() num_inputs = 0 - # get the set of param names + # get the set of param names param_names = set() for param in inputs: if "name" in param.attrib or "argument" in param.attrib: diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 6fd511b15549..3eada0eb9fb3 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -634,7 +634,7 @@ def load_from_parameter(from_parameter, transform_lines=None): data_file = data_file.strip() full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) full_path = os.path.normpath(full_path) - if safe_contains(self.tool_param.tool.app.config.tool_data_path, full_path) + if safe_contains(self.tool_param.tool.app.config.tool_data_path, full_path): if os.path.exists(full_path): self.index_file = data_file with open(full_path) as fh: From 5b7df0feb70519eb6436df33144cbe3b3fe0b0c5 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 19 Nov 2021 10:29:04 +0100 Subject: [PATCH 06/30] add link to tool-data/shared in test tool-data fix missing import --- lib/galaxy/tools/parameters/dynamic_options.py | 5 ++++- test/functional/tool-data/shared | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) create mode 120000 test/functional/tool-data/shared diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 3eada0eb9fb3..30f06b97b2ae 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -15,7 +15,10 @@ MetadataFile, User, ) -from galaxy.util import string_as_bool +from galaxy.util import ( + safe_contains, + string_as_bool +) from . import validation log = logging.getLogger(__name__) diff --git a/test/functional/tool-data/shared b/test/functional/tool-data/shared new file mode 120000 index 000000000000..888c0e444a46 --- /dev/null +++ b/test/functional/tool-data/shared @@ -0,0 +1 @@ +../../../tool-data/shared \ No newline at end of file From bce9300ae54870d963ed0812de7dec261e35e6f5 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 19 Nov 2021 17:11:25 +0100 Subject: [PATCH 07/30] remove debug messages --- lib/galaxy/tools/parameters/dynamic_options.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 30f06b97b2ae..604d40f6798b 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -567,7 +567,6 @@ def filter_options(self, options, trans, other_values): rval = [] for o in options: - log.error(f"o {o}") if self.keep == (o[self.column] in entries): rval.append(o) return rval @@ -642,7 +641,6 @@ def load_from_parameter(from_parameter, transform_lines=None): self.index_file = data_file with open(full_path) as fh: self.file_fields = self.parse_file_fields(fh) - log.error(f"exists file_fields {self.file_fields}") else: self.missing_index_file = data_file else: From 95bbd88f09660d071e30997f9659605d33234a47 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 6 Jun 2023 10:34:02 +0200 Subject: [PATCH 08/30] fix rebase mistake --- 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 26b24f9359fb..ce3842f84e38 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -1435,7 +1435,7 @@ def test_inputs_filter_correct(lint_ctx): assert not lint_ctx.error_messages -def test_inputs_filter_correct(lint_ctx): +def test_inputs_filter_incorrect(lint_ctx): tool_source = get_xml_tool_source(INPUTS_FILTER_INCORRECT) run_lint(lint_ctx, inputs.lint_inputs, tool_source) assert len(lint_ctx.info_messages) == 1 From 86331819bc03cb5162ee2b723f5adfa310dd1b48 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 6 Jun 2023 10:43:54 +0200 Subject: [PATCH 09/30] one more rebase error --- lib/galaxy/tool_util/xsd/galaxy.xsd | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index 7b9c033c996c..e16e4e2dc30a 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -5026,6 +5026,8 @@ from the list. Only used if ``type`` is ``attribute_value_splitter``. This is used to separate attributes and values from each other within an attribute-value pair, i.e. ``=`` if the target content is ``A=V; B=W; C=Y``. Defaults to whitespace. + + Only used when ``type`` is From d199a27d21c105cd290d2224790037f28c1e1ca2 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 6 Jun 2023 11:00:04 +0200 Subject: [PATCH 10/30] add context to new linter messages --- lib/galaxy/tool_util/linters/inputs.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 90a6f1c1506f..a2ebb9d6d19c 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -298,27 +298,27 @@ def lint_inputs(tool_xml, lint_ctx): # check for required attributes for filter (remove_value needs a bit more logic here) for attrib in FILTER_REQUIRED_ATTRIBUTES[ftype]: if attrib not in f.attrib: - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}' filter misses required attribute '{attrib}'") + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}' filter misses required attribute '{attrib}'", node=f) if ftype == "remove_value": if not (("value" in f.attrib and "ref" not in f.attrib and "meta_ref" not in f.attrib and "key" not in f.attrib) or ("value" not in f.attrib and "ref" in f.attrib and "meta_ref" not in f.attrib and "key" not in f.attrib) or ("value" not in f.attrib and "ref" not in f.attrib and "meta_ref" in f.attrib and "key" in f.attrib)): - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)") + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)", node=f) # check for allowed filter attributes (only warning because others are ignored) for attrib in f.attrib: if attrib not in FILTER_ALLOWED_ATTRIBUTES[ftype]: - lint_ctx.warn(f"Select parameter [{param_name}] '{ftype}' filter specifies unnecessary attribute '{attrib}'") + lint_ctx.warn(f"Select parameter [{param_name}] '{ftype}' filter specifies unnecessary attribute '{attrib}'", node=f) # check for references to other inputs # TODO: currently ref and metaref seem only to work for top level params, # once this is fixed the linter needs to be extended, e.g. `f.attrib[ref_attrib].split('|')[-1]` for ref_attrib in ["meta_ref", "ref"]: if ref_attrib in f.attrib and f.attrib[ref_attrib] not in param_names: - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter attribute '{ref_attrib}' refers to non existing parameter '{f.attrib[ref_attrib]}'") + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter attribute '{ref_attrib}' refers to non existing parameter '{f.attrib[ref_attrib]}'", node=f) if ftype == "regexp" and "value" in f.attrib: try: re.compile(f.attrib["value"]) except re.error as re_error: - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter 'value' is not a valid regular expression ({re_error})'") + lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter 'value' is not a valid regular expression ({re_error})'", node=f) from_file = options[0].get("from_file", None) from_parameter = options[0].get("from_parameter", None) From 38e60d6dbefb4bbdbea74bcee35ef35722c492b1 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 16 Feb 2024 15:49:48 +0100 Subject: [PATCH 11/30] add missing import --- lib/galaxy/tool_util/linters/inputs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 599891a5c45b..90bc12f8afcf 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -3,6 +3,7 @@ import ast import re from copy import deepcopy +from typing import TYPE_CHECKING from galaxy.util import string_as_bool from ._util import ( From c1bb7b5643b0d1845666a488e5878fbe2f339591 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Fri, 16 Feb 2024 15:58:43 +0100 Subject: [PATCH 12/30] update black formatting --- lib/galaxy/tool_util/linters/inputs.py | 50 +++++++++++++++---- .../tools/parameters/dynamic_options.py | 11 ++-- test/unit/tool_util/test_tool_linters.py | 25 ++++++++-- 3 files changed, 69 insertions(+), 17 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 90bc12f8afcf..7c62967a1eb1 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -27,7 +27,7 @@ "add_value", "remove_value", "sort_by", - "data_table" + "data_table", ] FILTER_REQUIRED_ATTRIBUTES = { @@ -314,27 +314,59 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): # check for required attributes for filter (remove_value needs a bit more logic here) for attrib in FILTER_REQUIRED_ATTRIBUTES[ftype]: if attrib not in f.attrib: - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}' filter misses required attribute '{attrib}'", node=f) + lint_ctx.error( + f"Select parameter [{param_name}] '{ftype}' filter misses required attribute '{attrib}'", + node=f, + ) if ftype == "remove_value": - if not (("value" in f.attrib and "ref" not in f.attrib and "meta_ref" not in f.attrib and "key" not in f.attrib) - or ("value" not in f.attrib and "ref" in f.attrib and "meta_ref" not in f.attrib and "key" not in f.attrib) - or ("value" not in f.attrib and "ref" not in f.attrib and "meta_ref" in f.attrib and "key" in f.attrib)): - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)", node=f) + if not ( + ( + "value" in f.attrib + and "ref" not in f.attrib + and "meta_ref" not in f.attrib + and "key" not in f.attrib + ) + or ( + "value" not in f.attrib + and "ref" in f.attrib + and "meta_ref" not in f.attrib + and "key" not in f.attrib + ) + or ( + "value" not in f.attrib + and "ref" not in f.attrib + and "meta_ref" in f.attrib + and "key" in f.attrib + ) + ): + lint_ctx.error( + f"Select parameter [{param_name}] '{ftype}'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)", + node=f, + ) # check for allowed filter attributes (only warning because others are ignored) for attrib in f.attrib: if attrib not in FILTER_ALLOWED_ATTRIBUTES[ftype]: - lint_ctx.warn(f"Select parameter [{param_name}] '{ftype}' filter specifies unnecessary attribute '{attrib}'", node=f) + lint_ctx.warn( + f"Select parameter [{param_name}] '{ftype}' filter specifies unnecessary attribute '{attrib}'", + node=f, + ) # check for references to other inputs # TODO: currently ref and metaref seem only to work for top level params, # once this is fixed the linter needs to be extended, e.g. `f.attrib[ref_attrib].split('|')[-1]` for ref_attrib in ["meta_ref", "ref"]: if ref_attrib in f.attrib and f.attrib[ref_attrib] not in param_names: - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter attribute '{ref_attrib}' refers to non existing parameter '{f.attrib[ref_attrib]}'", node=f) + lint_ctx.error( + f"Select parameter [{param_name}] '{ftype}'' filter attribute '{ref_attrib}' refers to non existing parameter '{f.attrib[ref_attrib]}'", + node=f, + ) if ftype == "regexp" and "value" in f.attrib: try: re.compile(f.attrib["value"]) except re.error as re_error: - lint_ctx.error(f"Select parameter [{param_name}] '{ftype}'' filter 'value' is not a valid regular expression ({re_error})'", node=f) + lint_ctx.error( + f"Select parameter [{param_name}] '{ftype}'' filter 'value' is not a valid regular expression ({re_error})'", + node=f, + ) from_file = options[0].get("from_file", None) from_parameter = options[0].get("from_parameter", None) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index dfdc69e8d4fa..e46669eee1d1 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -555,6 +555,7 @@ class DataTableFilter(Filter): Discard columns matching value (False) """ + def __init__(self, d_option, elem): Filter.__init__(self, d_option, elem) self.table_name = elem.get("table_name", None) @@ -564,17 +565,21 @@ def __init__(self, d_option, elem): self.column = d_option.column_spec_to_index(column) self.data_table_column = elem.get("data_table_column", None) assert self.data_table_column is not None, "Required 'data_table_column' attribute missing from filter" - self.keep = string_as_bool(elem.get("keep", 'True')) + self.keep = string_as_bool(elem.get("keep", "True")) def filter_options(self, options, trans, other_values): # get column from data table, by index or column name entries = None try: - entries = set([f[int(self.data_table_column)] for f in trans.app.tool_data_tables[self.table_name].get_fields()]) + entries = set( + [f[int(self.data_table_column)] for f in trans.app.tool_data_tables[self.table_name].get_fields()] + ) except TypeError: pass try: - entries = set([f[self.data_table_column] for f in trans.app.tool_data_tables[self.table_name].get_named_fields_list()]) + entries = set( + [f[self.data_table_column] for f in trans.app.tool_data_tables[self.table_name].get_named_fields_list()] + ) except KeyError: pass if entries is None: diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index c7971b8a1361..6fec63b1706a 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -1514,14 +1514,29 @@ def test_inputs_filter_incorrect(lint_ctx): run_lint(lint_ctx, inputs.lint_inputs, tool_source) assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages - assert "Select parameter [select_param] 'regexp' filter specifies unnecessary attribute 'multiple'" in lint_ctx.warn_messages + assert ( + "Select parameter [select_param] 'regexp' filter specifies unnecessary attribute 'multiple'" + in lint_ctx.warn_messages + ) assert len(lint_ctx.warn_messages) == 1 assert "Select parameter [select_param] contains filter without type." in lint_ctx.error_messages assert "Select parameter [select_param] contains filter with unknown type 'typo'." in lint_ctx.error_messages - assert "Select parameter [select_param] 'static_value' filter misses required attribute 'column'" in lint_ctx.error_messages - assert "Select parameter [select_param] 'remove_value'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)" in lint_ctx.error_messages - assert "Select parameter [select_param] 'data_meta'' filter attribute 'ref' refers to non existing parameter 'input'" in lint_ctx.error_messages - assert "Select parameter [select_param] 'regexp'' filter 'value' is not a valid regular expression (nothing to repeat at position 1)'" in lint_ctx.error_messages + assert ( + "Select parameter [select_param] 'static_value' filter misses required attribute 'column'" + in lint_ctx.error_messages + ) + assert ( + "Select parameter [select_param] 'remove_value'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)" + in lint_ctx.error_messages + ) + assert ( + "Select parameter [select_param] 'data_meta'' filter attribute 'ref' refers to non existing parameter 'input'" + in lint_ctx.error_messages + ) + assert ( + "Select parameter [select_param] 'regexp'' filter 'value' is not a valid regular expression (nothing to repeat at position 1)'" + in lint_ctx.error_messages + ) assert len(lint_ctx.error_messages) == 6 From c1091c1b5c5c270f1dd6122f4a8fdebcda60e21b Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Sat, 17 Feb 2024 15:59:38 +0100 Subject: [PATCH 13/30] update linters and tests to new style --- lib/galaxy/tool_util/linters/inputs.py | 211 ++++++++++++++++++++--- test/unit/tool_util/test_tool_linters.py | 27 +-- 2 files changed, 201 insertions(+), 37 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 8781c552d592..41a76b6c73da 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -3,6 +3,7 @@ import ast import re import warnings +from copy import deepcopy from typing import ( Iterator, Optional, @@ -28,31 +29,6 @@ lint_tool_types = ["*"] -FILTER_REQUIRED_ATTRIBUTES = { - "data_meta": ["type", "ref", "key"], # column needs special treatment - "param_value": ["type", "ref", "column"], - "static_value": ["type", "column", "value"], - "regexp": ["type", "column", "value"], - "unique_value": ["type", "column"], - "multiple_splitter": ["type", "column"], - "attribute_value_splitter": ["type", "column"], - "add_value": ["type", "value"], - "remove_value": ["type"], # this is handled separately - "sort_by": ["type", "column"], - "data_table": ["type", "column", "table_name", "data_table_column"], -} - -FILTER_ALLOWED_ATTRIBUTES = deepcopy(FILTER_REQUIRED_ATTRIBUTES) -FILTER_ALLOWED_ATTRIBUTES["static_value"].append("keep") -FILTER_ALLOWED_ATTRIBUTES["regexp"].append("keep") -FILTER_ALLOWED_ATTRIBUTES["data_meta"].extend(["column", "multiple", "separator"]) -FILTER_ALLOWED_ATTRIBUTES["param_value"].extend(["keep", "ref_attribute"]) -FILTER_ALLOWED_ATTRIBUTES["multiple_splitter"].append("separator") -FILTER_ALLOWED_ATTRIBUTES["attribute_value_splitter"].extend(["pair_separator", "name_val_separator"]) -FILTER_ALLOWED_ATTRIBUTES["add_value"].extend(["name", "index"]) -FILTER_ALLOWED_ATTRIBUTES["remove_value"].extend(["value", "ref", "meta_ref", "key"]) -FILTER_ALLOWED_ATTRIBUTES["data_table"].append("keep") - ATTRIB_VALIDATOR_COMPATIBILITY = { "check": ["metadata"], "expression": ["substitute_value_in_message"], @@ -152,7 +128,6 @@ ] # TODO lint for valid param type - attribute combinations -# TODO check if dataset is available for filters referring other datasets # TODO check if ref input param is present for from_dataset @@ -512,6 +487,190 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): ) +FILTER_REQUIRED_ATTRIBUTES = { + "data_meta": ["type", "ref", "key"], # column needs special treatment + "param_value": ["type", "ref", "column"], + "static_value": ["type", "column", "value"], + "regexp": ["type", "column", "value"], + "unique_value": ["type", "column"], + "multiple_splitter": ["type", "column"], + "attribute_value_splitter": ["type", "column"], + "add_value": ["type", "value"], + "remove_value": ["type"], # this is handled separately in InputsOptionsRemoveValueFilterRequiredAttributes + "sort_by": ["type", "column"], + "data_table": ["type", "column", "table_name", "data_table_column"], +} + + +class InputsOptionsFiltersRequiredAttributes(Linter): + """ + check required attributes of filters + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name in _iter_param(tool_xml): + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + filter_type = filter.get("type", None) + if filter_type is None or filter_type not in FILTER_ALLOWED_ATTRIBUTES: + continue + for attrib in FILTER_REQUIRED_ATTRIBUTES[filter_type]: + if attrib not in filter.attrib: + lint_ctx.error( + f"Select parameter [{param_name}] '{filter_type}' filter misses required attribute '{attrib}'", + node=filter, + ) + + +class InputsOptionsRemoveValueFilterRequiredAttributes(Linter): + """ + check required attributes of remove_value filter + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + for param, param_name in _iter_param(tool_xml): + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + filter_type = filter.get("type", None) + # check for required attributes for filter (remove_value needs a bit more logic here) + if filter_type != "remove_value": + continue + if not ( + ( + "value" in filter.attrib + and "ref" not in filter.attrib + and "meta_ref" not in filter.attrib + and "key" not in filter.attrib + ) + or ( + "value" not in filter.attrib + and "ref" in filter.attrib + and "meta_ref" not in filter.attrib + and "key" not in filter.attrib + ) + or ( + "value" not in filter.attrib + and "ref" not in filter.attrib + and "meta_ref" in filter.attrib + and "key" in filter.attrib + ) + ): + lint_ctx.error( + f"Select parameter [{param_name}] '{filter_type}'' filter needs either the 'value'; 'ref'; or 'meta' and 'key' attribute(s)", + node=filter, + ) + + +FILTER_ALLOWED_ATTRIBUTES = deepcopy(FILTER_REQUIRED_ATTRIBUTES) +FILTER_ALLOWED_ATTRIBUTES["static_value"].append("keep") +FILTER_ALLOWED_ATTRIBUTES["regexp"].append("keep") +FILTER_ALLOWED_ATTRIBUTES["data_meta"].extend(["column", "multiple", "separator"]) +FILTER_ALLOWED_ATTRIBUTES["param_value"].extend(["keep", "ref_attribute"]) +FILTER_ALLOWED_ATTRIBUTES["multiple_splitter"].append("separator") +FILTER_ALLOWED_ATTRIBUTES["attribute_value_splitter"].extend(["pair_separator", "name_val_separator"]) +FILTER_ALLOWED_ATTRIBUTES["add_value"].extend(["name", "index"]) +FILTER_ALLOWED_ATTRIBUTES["remove_value"].extend(["value", "ref", "meta_ref", "key"]) +FILTER_ALLOWED_ATTRIBUTES["data_table"].append("keep") + + +class InputsOptionsFiltersAllowedAttributes(Linter): + """ + check allowed attributes of filters + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + + for param, param_name in _iter_param(tool_xml): + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + filter_type = filter.get("type", None) + if filter_type is None or filter_type not in FILTER_ALLOWED_ATTRIBUTES: + continue + for attrib in filter.attrib: + if attrib not in FILTER_ALLOWED_ATTRIBUTES[filter_type]: + lint_ctx.warn( + f"Select parameter [{param_name}] '{filter_type}' filter specifies unnecessary attribute '{attrib}'", + node=filter, + ) + + +class InputsOptionsRegexFilterExpression(Linter): + """ + Check the regular expression of regexp filters + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + + for param, param_name in _iter_param(tool_xml): + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + filter_type = filter.get("type", None) + if filter_type == "regexp" and "value" in filter.attrib: + try: + re.compile(filter.attrib["value"]) + except re.error as re_error: + lint_ctx.error( + f"Select parameter [{param_name}] '{filter_type}'' filter 'value' is not a valid regular expression ({re_error})'", + node=filter, + ) + + +class InputsOptionsFiltersCheckReferences(Linter): + """ + Check the references used in filters + """ + + @classmethod + def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): + tool_xml = getattr(tool_source, "xml_tree", None) + if not tool_xml: + return + + # get the set of param names + param_names = set([param_name for _, param_name in _iter_param(tool_xml)]) + + for param, param_name in _iter_param(tool_xml): + options = param.find("./options") + if options is None: + continue + for filter in param.findall("./options/filter"): + filter_type = filter.get("type", None) + if filter_type is not None: + # check for references to other inputs + # TODO: currently ref and metaref seem only to work for top level params, + # once this is fixed the linter needs to be extended, e.g. `f.attrib[ref_attrib].split('|')[-1]` + for ref_attrib in ["meta_ref", "ref"]: + if ref_attrib in filter.attrib and filter.attrib[ref_attrib] not in param_names: + lint_ctx.error( + f"Select parameter [{param_name}] '{filter_type}'' filter attribute '{ref_attrib}' refers to non existing parameter '{filter.attrib[ref_attrib]}'", + node=filter, + ) + + class InputsDataOptionsFiltersRef(Linter): """ Lint for set ref for filters of data parameters diff --git a/test/unit/tool_util/test_tool_linters.py b/test/unit/tool_util/test_tool_linters.py index d1a7171ad457..6467d5fb1791 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -204,6 +204,7 @@ INPUTS_DATA_PARAM_OPTIONS = """ + @@ -216,6 +217,7 @@ INPUTS_DATA_PARAM_OPTIONS_FILTER_ATTRIBUTE = """ + @@ -231,7 +233,7 @@ - + @@ -523,7 +525,7 @@ """ INPUTS_FILTER_INCORRECT = """ - + @@ -541,7 +543,7 @@ """ INPUTS_FILTER_CORRECT = """ - + @@ -1268,7 +1270,7 @@ def test_inputs_data_param_options(lint_ctx): tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_OPTIONS) run_lint_module(lint_ctx, inputs, tool_source) assert not lint_ctx.valid_messages - assert "Found 1 input parameters." in lint_ctx.info_messages + assert "Found 2 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 @@ -1278,7 +1280,7 @@ def test_inputs_data_param_options_filter_attribute(lint_ctx): tool_source = get_xml_tool_source(INPUTS_DATA_PARAM_OPTIONS_FILTER_ATTRIBUTE) run_lint_module(lint_ctx, inputs, tool_source) assert not lint_ctx.valid_messages - assert "Found 1 input parameters." in lint_ctx.info_messages + assert "Found 2 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 @@ -1430,7 +1432,7 @@ def test_inputs_select_filter(lint_ctx): assert "Found 1 input parameters." in lint_ctx.info_messages assert "Invalid XML: Element 'filter': The attribute 'type' is required but missing." in lint_ctx.error_messages assert ( - "Invalid XML: Element 'filter', attribute 'type': [facet 'enumeration'] The value 'unknown_filter_type' is not an element of the set {'data_meta', 'param_value', 'static_value', 'regexp', 'unique_value', 'multiple_splitter', 'attribute_value_splitter', 'add_value', 'remove_value', 'sort_by'}." + "Invalid XML: Element 'filter', attribute 'type': [facet 'enumeration'] The value 'unknown_filter_type' is not an element of the set {'data_meta', 'param_value', 'static_value', 'regexp', 'unique_value', 'multiple_splitter', 'attribute_value_splitter', 'add_value', 'remove_value', 'sort_by', 'data_table'}." in lint_ctx.error_messages ) assert len(lint_ctx.info_messages) == 1 @@ -1535,7 +1537,7 @@ def test_inputs_duplicate_names(lint_ctx): def test_inputs_filter_correct(lint_ctx): tool_source = get_xml_tool_source(INPUTS_FILTER_CORRECT) - run_lint(lint_ctx, inputs.lint_inputs, tool_source) + run_lint_module(lint_ctx, inputs, tool_source) assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages assert not lint_ctx.warn_messages @@ -1544,7 +1546,7 @@ def test_inputs_filter_correct(lint_ctx): def test_inputs_filter_incorrect(lint_ctx): tool_source = get_xml_tool_source(INPUTS_FILTER_INCORRECT) - run_lint(lint_ctx, inputs.lint_inputs, tool_source) + run_lint_module(lint_ctx, inputs, tool_source) assert len(lint_ctx.info_messages) == 1 assert not lint_ctx.valid_messages assert ( @@ -1552,8 +1554,11 @@ def test_inputs_filter_incorrect(lint_ctx): in lint_ctx.warn_messages ) assert len(lint_ctx.warn_messages) == 1 - assert "Select parameter [select_param] contains filter without type." in lint_ctx.error_messages - assert "Select parameter [select_param] contains filter with unknown type 'typo'." in lint_ctx.error_messages + assert "Invalid XML: Element 'filter': The attribute 'type' is required but missing." in lint_ctx.error_messages + assert ( + "Invalid XML: Element 'filter', attribute 'type': [facet 'enumeration'] The value 'typo' is not an element of the set {'data_meta', 'param_value', 'static_value', 'regexp', 'unique_value', 'multiple_splitter', 'attribute_value_splitter', 'add_value', 'remove_value', 'sort_by', 'data_table'}." + in lint_ctx.error_messages + ) assert ( "Select parameter [select_param] 'static_value' filter misses required attribute 'column'" in lint_ctx.error_messages @@ -2158,7 +2163,7 @@ def test_xml_comments_are_ignored(lint_ctx: LintContext): def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 129 + assert len(linter_names) == 134 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From e8a612b3f35318b9d6b296c4303bea39909377f2 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 22 Oct 2024 13:30:24 +0200 Subject: [PATCH 14/30] remove unnecessary list comprehensions --- lib/galaxy/tool_util/linters/inputs.py | 2 +- lib/galaxy/tools/parameters/dynamic_options.py | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 3ca31009c462..815c1a6d0142 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -654,7 +654,7 @@ def lint(cls, tool_source: "ToolSource", lint_ctx: "LintContext"): return # get the set of param names - param_names = set([param_name for _, param_name in _iter_param(tool_xml)]) + param_names = {param_name for _, param_name in _iter_param(tool_xml)} for param, param_name in _iter_param(tool_xml): options = param.find("./options") diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 145efb89c077..0a5ef3726a8a 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -573,15 +573,13 @@ def filter_options(self, options, trans, other_values): # get column from data table, by index or column name entries = None try: - entries = set( - [f[int(self.data_table_column)] for f in trans.app.tool_data_tables[self.table_name].get_fields()] - ) + entries = {f[int(self.data_table_column)] for f in trans.app.tool_data_tables[self.table_name].get_fields()} except TypeError: pass try: - entries = set( - [f[self.data_table_column] for f in trans.app.tool_data_tables[self.table_name].get_named_fields_list()] - ) + entries = { + f[self.data_table_column] for f in trans.app.tool_data_tables[self.table_name].get_named_fields_list() + } except KeyError: pass if entries is None: From a6646385af914191e92bc5d8dd993c765b71c716 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 22 Oct 2024 13:48:50 +0200 Subject: [PATCH 15/30] fix number of tests --- 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 468feeece79c..1db34ff1bcb8 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -2232,7 +2232,7 @@ def test_skip_by_module(lint_ctx): def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 135 + assert len(linter_names) == 137 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From cde25edf07163a2388ea08f9ca9657a9a21ee8a1 Mon Sep 17 00:00:00 2001 From: M Bernt Date: Wed, 6 Nov 2024 14:49:01 +0100 Subject: [PATCH 16/30] Update test/unit/tool_util/test_tool_linters.py --- 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 8af3d7d20c1c..52c4ee6341eb 100644 --- a/test/unit/tool_util/test_tool_linters.py +++ b/test/unit/tool_util/test_tool_linters.py @@ -2289,7 +2289,7 @@ def test_skip_by_module(lint_ctx): def test_list_linters(): linter_names = Linter.list_listers() # make sure to add/remove a test for new/removed linters if this number changes - assert len(linter_names) == 137 + assert len(linter_names) == 139 assert "Linter" not in linter_names # make sure that linters from all modules are available for prefix in [ From c595c0eed10ac4ef43647a828811b73e215bdbd3 Mon Sep 17 00:00:00 2001 From: M Bernt Date: Wed, 13 Nov 2024 15:45:44 +0100 Subject: [PATCH 17/30] Remove `normpath` call --- lib/galaxy/tools/parameters/dynamic_options.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 75c4b3662668..67ff6af78730 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -657,7 +657,6 @@ def load_from_parameter(from_parameter, transform_lines=None): if data_file is not None: data_file = data_file.strip() full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) - full_path = os.path.normpath(full_path) if safe_contains(self.tool_param.tool.app.config.tool_data_path, full_path): if os.path.exists(full_path): self.index_file = data_file From 001c1dd300a265b61ab9cde605af7e567207ddcd Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 13 Nov 2024 21:55:18 +0100 Subject: [PATCH 18/30] revert some changes --- lib/galaxy/tools/parameters/dynamic_options.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 75c4b3662668..592dbd9cf575 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -656,18 +656,14 @@ def load_from_parameter(from_parameter, transform_lines=None): self.parse_column_definitions(elem) if data_file is not None: data_file = data_file.strip() - full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) - full_path = os.path.normpath(full_path) - if safe_contains(self.tool_param.tool.app.config.tool_data_path, full_path): + if not os.path.isabs(data_file): + full_path = os.path.join(self.tool_param.tool.app.config.tool_data_path, data_file) if os.path.exists(full_path): self.index_file = data_file with open(full_path) as fh: self.file_fields = self.parse_file_fields(fh) else: self.missing_index_file = data_file - else: - log.error(f"'from_file' ({data_file}) references path outside of Galaxy's tool-data dir!") - self.missing_index_file = data_file elif dataset_file is not None: self.meta_file_key = elem.get("meta_file_key", None) self.dataset_ref_name = dataset_file From b926ca0a56ed477fa15d377942404567ab0eb055 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 13 Nov 2024 22:13:58 +0100 Subject: [PATCH 19/30] try to adapt test from_file does not work (relative paths are not possible .. and in the tests we do not know the abs path) --- test/functional/tools/filter_data_table.xml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/functional/tools/filter_data_table.xml b/test/functional/tools/filter_data_table.xml index c6e67b27b82b..32c3c98c4169 100644 --- a/test/functional/tools/filter_data_table.xml +++ b/test/functional/tools/filter_data_table.xml @@ -6,13 +6,15 @@ - + - + + @@ -23,7 +25,7 @@ - + @@ -32,7 +34,7 @@ - + From e851a45ec91e1591f8cb383f264311f6f9c0c43f Mon Sep 17 00:00:00 2001 From: M Bernt Date: Wed, 13 Nov 2024 22:19:50 +0100 Subject: [PATCH 20/30] Update lib/galaxy/tools/parameters/dynamic_options.py --- lib/galaxy/tools/parameters/dynamic_options.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index 592dbd9cf575..f738c4f9b495 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -34,7 +34,6 @@ ) from galaxy.util import ( Element, - safe_contains, string_as_bool, ) from galaxy.util.template import fill_template From 174f24406f0d0d9cb41f34de5f5ea87ea468efeb Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 14 Nov 2024 13:33:56 +0100 Subject: [PATCH 21/30] fix test --- test/functional/tools/filter_data_table.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/tools/filter_data_table.xml b/test/functional/tools/filter_data_table.xml index 32c3c98c4169..1943c4df1ca4 100644 --- a/test/functional/tools/filter_data_table.xml +++ b/test/functional/tools/filter_data_table.xml @@ -28,7 +28,7 @@ - + @@ -37,7 +37,7 @@ - + From 157d074c68cb8c1da4659f6fa0906bd6ae317086 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 14 Nov 2024 15:05:33 +0100 Subject: [PATCH 22/30] remove output assertion for failing test `Exception: Cannot specify outputs in a test expecting failure.` --- test/functional/tools/filter_data_table.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/functional/tools/filter_data_table.xml b/test/functional/tools/filter_data_table.xml index 1943c4df1ca4..37908d5fd081 100644 --- a/test/functional/tools/filter_data_table.xml +++ b/test/functional/tools/filter_data_table.xml @@ -35,11 +35,6 @@ - - - - - From 267603953ac82a1ede06ecc1724d801912a6a9cc Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Thu, 14 Nov 2024 17:15:24 +0100 Subject: [PATCH 23/30] fix test --- test/functional/tools/filter_data_table.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/tools/filter_data_table.xml b/test/functional/tools/filter_data_table.xml index 37908d5fd081..5576e6147357 100644 --- a/test/functional/tools/filter_data_table.xml +++ b/test/functional/tools/filter_data_table.xml @@ -10,7 +10,7 @@ - + - +