From 8717c4c49106ed456c0030fcdafcc9b9b1304517 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Tue, 12 Dec 2023 11:14:48 +0100 Subject: [PATCH] make profile a Version --- lib/galaxy/tool_util/deps/container_classes.py | 2 +- lib/galaxy/tool_util/deps/dependencies.py | 4 +++- lib/galaxy/tool_util/linters/general.py | 4 ++-- lib/galaxy/tool_util/linters/inputs.py | 2 +- lib/galaxy/tool_util/linters/outputs.py | 4 ++-- lib/galaxy/tool_util/linters/stdio.py | 2 +- lib/galaxy/tool_util/parser/cwl.py | 8 ++++---- lib/galaxy/tool_util/parser/interface.py | 6 +++--- lib/galaxy/tool_util/parser/xml.py | 18 +++++++++--------- lib/galaxy/tools/__init__.py | 13 ++++++------- lib/galaxy/tools/actions/__init__.py | 4 ++-- lib/galaxy/tools/evaluation.py | 2 +- lib/galaxy/tools/execute.py | 2 +- lib/galaxy/tools/parameters/basic.py | 4 ++-- lib/galaxy/tools/wrappers.py | 4 ++-- test/unit/app/tools/test_evaluation.py | 4 +++- test/unit/app/tools/test_wrappers.py | 3 ++- 17 files changed, 45 insertions(+), 41 deletions(-) diff --git a/lib/galaxy/tool_util/deps/container_classes.py b/lib/galaxy/tool_util/deps/container_classes.py index f55f78614f4f..e7fdb0d4bd92 100644 --- a/lib/galaxy/tool_util/deps/container_classes.py +++ b/lib/galaxy/tool_util/deps/container_classes.py @@ -364,7 +364,7 @@ def add_var(name, value): defaults += ",$tool_directory:default_ro" if self.job_info.job_directory: defaults += ",$job_directory:default_ro,$job_directory/outputs:rw" - if Version(str(self.tool_info.profile)) <= Version("19.09"): + if self.tool_info.profile <= Version("19.09"): defaults += ",$job_directory/configs:rw" if self.job_info.home_directory is not None: defaults += ",$home_directory:rw" diff --git a/lib/galaxy/tool_util/deps/dependencies.py b/lib/galaxy/tool_util/deps/dependencies.py index cb5d571b44d5..bc097edeb90f 100644 --- a/lib/galaxy/tool_util/deps/dependencies.py +++ b/lib/galaxy/tool_util/deps/dependencies.py @@ -6,6 +6,8 @@ Union, ) +from packaging.version import Version + from galaxy.tool_util.deps.requirements import ( ContainerDescription, ToolRequirement, @@ -62,7 +64,7 @@ def __init__( guest_ports=None, tool_id: Optional[str] = None, tool_version: Optional[str] = None, - profile: float = -1, + profile: Version = Version("0"), ): if env_pass_through is None: env_pass_through = ["GALAXY_SLOTS", "GALAXY_MEMORY_MB", "GALAXY_MEMORY_MB_PER_SLOT"] diff --git a/lib/galaxy/tool_util/linters/general.py b/lib/galaxy/tool_util/linters/general.py index d9b06c121725..a1e97cd8c7fa 100644 --- a/lib/galaxy/tool_util/linters/general.py +++ b/lib/galaxy/tool_util/linters/general.py @@ -66,10 +66,10 @@ def lint_general(tool_source, lint_ctx): lint_ctx.valid(VALID_ID_MSG % tool_id, node=tool_node) profile = tool_source.parse_profile() - profile_valid = PROFILE_PATTERN.match(profile) is not None + profile_valid = PROFILE_PATTERN.match(str(profile)) is not None if not profile_valid: lint_ctx.error(PROFILE_INVALID_MSG % profile, node=tool_node) - elif Version(profile) == Version("16.01"): + elif profile == Version("16.01"): lint_ctx.valid(PROFILE_INFO_DEFAULT_MSG, node=tool_node) else: lint_ctx.valid(PROFILE_INFO_SPECIFIED_MSG % profile, node=tool_node) diff --git a/lib/galaxy/tool_util/linters/inputs.py b/lib/galaxy/tool_util/linters/inputs.py index 7dd46c21f0e9..47e3e08ecc6b 100644 --- a/lib/galaxy/tool_util/linters/inputs.py +++ b/lib/galaxy/tool_util/linters/inputs.py @@ -346,7 +346,7 @@ def lint_inputs(tool_source: "ToolSource", lint_ctx: "LintContext"): if param_type == "boolean": truevalue = param_attrib.get("truevalue", "true") falsevalue = param_attrib.get("falsevalue", "false") - problematic_booleans_allowed = Version(profile) < Version("23.1") + problematic_booleans_allowed = profile < Version("23.1") lint_level = lint_ctx.warn if problematic_booleans_allowed else lint_ctx.error if truevalue == falsevalue: lint_level( diff --git a/lib/galaxy/tool_util/linters/outputs.py b/lib/galaxy/tool_util/linters/outputs.py index e034b0fe606d..4ec83ae8ff48 100644 --- a/lib/galaxy/tool_util/linters/outputs.py +++ b/lib/galaxy/tool_util/linters/outputs.py @@ -96,7 +96,7 @@ def lint_output(tool_source: "ToolSource", lint_ctx: "LintContext"): lint_ctx.info(f"{num_outputs} outputs found.", node=outputs[0]) -def __check_format(node, lint_ctx, profile: str, allow_ext=False): +def __check_format(node, lint_ctx, profile: Version, allow_ext=False): """ check if format/ext/format_source attribute is set in a given node issue a warning if the value is input @@ -119,7 +119,7 @@ def __check_format(node, lint_ctx, profile: str, allow_ext=False): fmt = node.attrib.get("format") if fmt == "input": message = f"Using format='input' on {node.tag} is deprecated. Use the format_source attribute." - if Version(str(profile)) <= Version("16.01"): + if profile <= Version("16.01"): lint_ctx.warn(message, node=node) else: lint_ctx.error(message, node=node) diff --git a/lib/galaxy/tool_util/linters/stdio.py b/lib/galaxy/tool_util/linters/stdio.py index ac1171b49eea..16777b5d1497 100644 --- a/lib/galaxy/tool_util/linters/stdio.py +++ b/lib/galaxy/tool_util/linters/stdio.py @@ -22,7 +22,7 @@ def lint_stdio(tool_source, lint_ctx): if not stdios: command = get_command(tool_xml) if tool_xml else None if command is None or not command.get("detect_errors"): - if Version(tool_source.parse_profile()) <= Version("16.01"): + if tool_source.parse_profile() <= Version("16.01"): lint_ctx.info( "No stdio definition found, tool indicates error conditions with output written to stderr.", node=tool_node, diff --git a/lib/galaxy/tool_util/parser/cwl.py b/lib/galaxy/tool_util/parser/cwl.py index 1647c39aa701..a50b322d5276 100644 --- a/lib/galaxy/tool_util/parser/cwl.py +++ b/lib/galaxy/tool_util/parser/cwl.py @@ -3,7 +3,7 @@ import math from typing import Optional -import packaging.version +from packaging.version import Version from galaxy.tool_util.cwl.parser import ( tool_proxy, @@ -163,8 +163,8 @@ def parse_requirements_and_containers(self): resource_requirements=resource_requirements, ) - def parse_profile(self): - return "17.09" + def parse_profile(self) -> Version: + return Version("17.09") def parse_xrefs(self): return [] @@ -173,7 +173,7 @@ def parse_license(self): return None def parse_python_template_version(self): - return packaging.version.Version("3.5") + return Version("3.5") def to_string(self): return json.dumps(self.tool_proxy.to_persistent_representation()) diff --git a/lib/galaxy/tool_util/parser/interface.py b/lib/galaxy/tool_util/parser/interface.py index f4c65c13dd0c..0009117318cd 100644 --- a/lib/galaxy/tool_util/parser/interface.py +++ b/lib/galaxy/tool_util/parser/interface.py @@ -16,7 +16,7 @@ Union, ) -import packaging.version +from packaging.version import Version from typing_extensions import TypedDict from galaxy.util.path import safe_walk @@ -276,7 +276,7 @@ def parse_help(self) -> Optional[str]: """ @abstractmethod - def parse_profile(self): + def parse_profile(self) -> Version: """Return tool profile version as Galaxy major e.g. 16.01 or 16.04.""" @abstractmethod @@ -284,7 +284,7 @@ def parse_license(self): """Return license corresponding to tool wrapper.""" @abstractmethod - def parse_python_template_version(self) -> Optional[packaging.version.Version]: + def parse_python_template_version(self) -> Optional[Version]: """ Return minimum python version that the tool template has been developed against. """ diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 1b7c91755f54..4706608fc5c3 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -130,7 +130,7 @@ def __init__(self, xml_tree: ElementTree, source_path=None, macro_paths=None): self.root = self.xml_tree.getroot() self._source_path = source_path self._macro_paths = macro_paths or [] - self.legacy_defaults = Version(self.parse_profile()) == Version("16.01") + self.legacy_defaults = self.parse_profile() == Version("16.01") self._string = xml_to_string(self.root) def to_string(self): @@ -250,7 +250,7 @@ def parse_environment_variables(self): return environment_variables def parse_home_target(self): - target = "job_home" if Version(self.parse_profile()) >= Version("18.01") else "shared_home" + target = "job_home" if self.parse_profile() >= Version("18.01") else "shared_home" command_el = self._command_el command_legacy = (command_el is not None) and command_el.get("use_shared_home", None) if command_legacy is not None: @@ -397,7 +397,7 @@ def parse_provided_metadata_style(self): style = out_elem.attrib["provided_metadata_style"] if style is None: - style = "legacy" if Version(self.parse_profile()) < Version("17.09") else "default" + style = "legacy" if self.parse_profile() < Version("17.09") else "default" assert style in ["legacy", "default"] return style @@ -539,7 +539,7 @@ def _parse_output( output.filters = data_elem.findall("filter") output.tool = tool output.from_work_dir = data_elem.get("from_work_dir", None) - if output.from_work_dir and Version(str(getattr(tool, "profile", 0))) < Version("21.09"): + if output.from_work_dir and getattr(tool, "profile", Version("16.01")) < Version("21.09"): # We started quoting from_work_dir outputs in 21.09. # Prior to quoting, trailing spaces had no effect. # This ensures that old tools continue to work. @@ -616,7 +616,7 @@ def parse_stdio(self): def parse_strict_shell(self): command_el = self._command_el - if Version(self.parse_profile()) < Version("20.09"): + if self.parse_profile() < Version("20.09"): default = "False" else: default = "True" @@ -649,14 +649,14 @@ def parse_tests_to_dict(self) -> ToolSourceTests: return rval - def parse_profile(self) -> str: + def parse_profile(self) -> Version: # Pre-16.04 or default XML defaults # - Use standard error for error detection. # - Don't run shells with -e # - Auto-check for implicit multiple outputs. # - Auto-check for $param_file. # - Enable buggy interpreter attribute. - return self.root.get("profile", "16.01") + return Version(self.root.get("profile", "16.01")) def parse_license(self): return self.root.get("license") @@ -687,7 +687,7 @@ def parse_creator(self): return creators -def _test_elem_to_dict(test_elem, i, profile=None) -> ToolSourceTest: +def _test_elem_to_dict(test_elem, i, profile: Optional[Version] = None) -> ToolSourceTest: rval: ToolSourceTest = dict( outputs=__parse_output_elems(test_elem), output_collections=__parse_output_collection_elems(test_elem, profile=profile), @@ -760,7 +760,7 @@ def __parse_element_tests(parent_element, profile=None): element_tests[identifier] = __parse_test_attributes( element, element_attrib, parse_elements=True, profile=profile ) - if profile and Version(profile) >= Version("20.09"): + if profile and profile >= Version("20.09"): element_tests[identifier][1]["expected_sort_order"] = idx return element_tests diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 27aa588cd831..9112f87f91f7 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -910,10 +910,10 @@ def requires_galaxy_python_environment(self): if self.tool_type not in ["default", "manage_data", "interactive", "data_source"]: return True - if self.tool_type == "manage_data" and Version(str(self.profile)) < Version("18.09"): + if self.tool_type == "manage_data" and self.profile < Version("18.09"): return True - if self.tool_type == "data_source" and Version(str(self.profile)) < Version("21.09"): + if self.tool_type == "data_source" and self.profile < Version("21.09"): return True config = self.app.config @@ -997,7 +997,7 @@ def parse(self, tool_source: ToolSource, guid=None, dynamic=False): """ Read tool configuration from the element `root` and fill in `self`. """ - self.profile = float(tool_source.parse_profile()) + self.profile = tool_source.parse_profile() # Get the UNIQUE id for the tool self.old_id = tool_source.parse_id() if guid is None: @@ -1008,15 +1008,14 @@ def parse(self, tool_source: ToolSource, guid=None, dynamic=False): if not dynamic and not self.id: raise Exception(f"Missing tool 'id' for tool at '{tool_source}'") - profile = Version(str(self.profile)) - if self.app.name == "galaxy" and profile >= Version("16.04") and Version(VERSION_MAJOR) < profile: + if self.app.name == "galaxy" and self.profile >= Version("16.04") and Version(VERSION_MAJOR) < self.profile: message = f"The tool [{self.id}] targets version {self.profile} of Galaxy, you should upgrade Galaxy to ensure proper functioning of this tool." raise Exception(message) self.python_template_version = tool_source.parse_python_template_version() if self.python_template_version is None: # If python_template_version not specified we assume tools with profile versions >= 19.05 are python 3 ready - if profile >= Version("19.05"): + if self.profile >= Version("19.05"): self.python_template_version = Version("3.5") else: self.python_template_version = Version("2.7") @@ -1030,7 +1029,7 @@ def parse(self, tool_source: ToolSource, guid=None, dynamic=False): self.version = tool_source.parse_version() if not self.version: - if profile < Version("16.04"): + if self.profile < Version("16.04"): # For backward compatibility, some tools may not have versions yet. self.version = "1.0.0" else: diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 54d71b35d995..c4e887333e8e 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -419,7 +419,7 @@ def execute( # format='input" previously would give you a random extension from # the input extensions, now it should just give "input" as the output # format. - input_ext = "data" if Version(str(tool.profile)) < Version("16.04") else "input" + input_ext = "data" if tool.profile < Version("16.04") else "input" input_dbkey = incoming.get("dbkey", "?") for name, data in reversed(list(inp_data.items())): if not data: @@ -431,7 +431,7 @@ def execute( data = data.to_history_dataset_association(None) inp_data[name] = data - if Version(str(tool.profile)) < Version("16.04"): + if tool.profile < Version("16.04"): input_ext = data.ext if data.dbkey not in [None, "?"]: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 4fbc27e4b67d..ec62b0f7a7d4 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -728,7 +728,7 @@ def _build_param_file(self): param_dict = self.param_dict directory = self.local_working_directory command = self.tool.command - if Version(str(self.tool.profile)) < Version("16.04") and command and "$param_file" in command: + if self.tool.profile < Version("16.04") and command and "$param_file" in command: with tempfile.NamedTemporaryFile(mode="w", dir=directory, delete=False) as param: for key, value in param_dict.items(): # parameters can be strings or lists of strings, coerce to list diff --git a/lib/galaxy/tools/execute.py b/lib/galaxy/tools/execute.py index 052c5f2fb84b..a20d7b258d46 100644 --- a/lib/galaxy/tools/execute.py +++ b/lib/galaxy/tools/execute.py @@ -310,7 +310,7 @@ def output_name(self, trans, history, params, output): return output_collection_name def sliced_input_collection_structure(self, input_name): - unqualified_recurse = Version(str(self.tool.profile)) < Version("18.09") and "|" not in input_name + unqualified_recurse = self.tool.profile < Version("18.09") and "|" not in input_name def find_collection(input_dict, input_name): for key, value in input_dict.items(): diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 897573d5fed4..683312fb61bf 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -593,7 +593,7 @@ def __init__(self, tool, input_source): super().__init__(tool, input_source) truevalue = input_source.get("truevalue", "true") falsevalue = input_source.get("falsevalue", "false") - if tool and Version(str(tool.profile)) >= Version("23.1"): + if tool and tool.profile >= Version("23.1"): if truevalue == falsevalue: raise ParameterValueError("Cannot set true and false to the same value", self.name) if truevalue.lower() == "false": @@ -1021,7 +1021,7 @@ def from_json(self, value, trans, other_values=None, require_legal_value=True): "an invalid option (None) was selected, please verify", self.name, None, is_dynamic=self.is_dynamic ) elif not legal_values: - if self.optional and Version(str(self.tool.profile)) < Version("18.09"): + if self.optional and self.tool.profile < Version("18.09"): # Covers optional parameters with default values that reference other optional parameters. # These will have a value but no legal_values. # See https://github.com/galaxyproject/tools-iuc/pull/1842#issuecomment-394083768 for context. diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index fb1dd32dc1a9..de1cb456b8f8 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -122,7 +122,7 @@ def __init__( input: "ToolParameter", value: Optional[str], other_values: Optional[Dict[str, str]] = None, - profile: Optional[float] = None, + profile: Optional[Version] = None, ) -> None: self.input = input if ( @@ -130,7 +130,7 @@ def __init__( and input.type == "text" and input.optional and input.optionality_inferred - and (profile is None or Version(str(profile)) < Version("23.0")) + and (profile is None or profile < Version("23.0")) ): # Tools with old profile versions may treat an optional text parameter as `""` value = "" diff --git a/test/unit/app/tools/test_evaluation.py b/test/unit/app/tools/test_evaluation.py index 24473b5d7fcf..6ebbc4651be7 100644 --- a/test/unit/app/tools/test_evaluation.py +++ b/test/unit/app/tools/test_evaluation.py @@ -1,5 +1,7 @@ import os +from packaging.version import Version + from galaxy.app_unittest_utils.tools_support import UsesApp from galaxy.job_execution.compute_environment import SimpleComputeEnvironment from galaxy.job_execution.datasets import DatasetPath @@ -297,7 +299,7 @@ def get_file_sources_dict(self): class MockTool: def __init__(self, app): - self.profile = 16.01 + self.profile = Version("16.01") self.python_template_version = "2.7" self.app = app self.hooks_called = [] diff --git a/test/unit/app/tools/test_wrappers.py b/test/unit/app/tools/test_wrappers.py index a7be2e271d1b..286e9da24266 100644 --- a/test/unit/app/tools/test_wrappers.py +++ b/test/unit/app/tools/test_wrappers.py @@ -4,6 +4,7 @@ from unittest.mock import Mock import pytest +from packaging.version import Version from galaxy.datatypes.metadata import MetadataSpecCollection from galaxy.job_execution.compute_environment import ComputeEnvironment @@ -320,7 +321,7 @@ class MockTool: def __init__(self, app): self.app = app self.options = Mock(sanitize=False) - self.profile = 23.0 + self.profile = Version("23.0") class MockApp: