From 5103a2d3964ed89be8064834321419438a046b12 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 14:34:59 +0100 Subject: [PATCH 01/18] WIP: unused variables --- lib/galaxy/jobs/runners/state_handlers/_safe_eval.py | 2 -- lib/galaxy/tool_util/deps/conda_compat.py | 5 ----- lib/galaxy/tool_util/deps/docker_util.py | 2 +- lib/galaxy/tool_util/linters/xml_order.py | 4 +++- lib/galaxy/tool_util/parser/output_actions.py | 2 +- lib/galaxy/tool_util/toolbox/base.py | 2 +- lib/galaxy/tool_util/verify/__init__.py | 6 ++---- lib/galaxy/tools/cache.py | 10 ---------- lib/galaxy/util/__init__.py | 6 +++--- lib/tool_shed/test/base/twilltestcase.py | 5 ++++- mypy.ini | 2 ++ 11 files changed, 17 insertions(+), 29 deletions(-) diff --git a/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py b/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py index 6c04fede96f8..e43292084e2c 100644 --- a/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py +++ b/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py @@ -127,8 +127,6 @@ def _check_expression(text, allowed_variables=None): except SyntaxError: return False - if not isinstance(module, Module): - return False statements = module.body if not len(statements) == 1: return False diff --git a/lib/galaxy/tool_util/deps/conda_compat.py b/lib/galaxy/tool_util/deps/conda_compat.py index d4ee224960cb..a7f3dd9dd92d 100644 --- a/lib/galaxy/tool_util/deps/conda_compat.py +++ b/lib/galaxy/tool_util/deps/conda_compat.py @@ -6,7 +6,6 @@ """ import os -from collections.abc import Hashable import yaml @@ -31,10 +30,6 @@ def __init__(self, func): self.cache = {} def __call__(self, *args): - if not isinstance(args, Hashable): - # uncacheable. a list, for instance. - # better to not cache than blow up. - return self.func(*args) if args in self.cache: return self.cache[args] else: diff --git a/lib/galaxy/tool_util/deps/docker_util.py b/lib/galaxy/tool_util/deps/docker_util.py index c47a3296bbda..96c7e29f38d1 100644 --- a/lib/galaxy/tool_util/deps/docker_util.py +++ b/lib/galaxy/tool_util/deps/docker_util.py @@ -110,7 +110,7 @@ def build_docker_run_command( auto_rm: bool = DEFAULT_AUTO_REMOVE, set_user: Optional[str] = DEFAULT_SET_USER, host: Optional[str] = DEFAULT_HOST, - guest_ports: Union[bool, List[str]] = False, + guest_ports: Union[bool, str, List[str]] = False, container_name: Optional[str] = None, ) -> str: env_directives = env_directives or [] diff --git a/lib/galaxy/tool_util/linters/xml_order.py b/lib/galaxy/tool_util/linters/xml_order.py index 7fb2bd261331..7ae29cd21dbf 100644 --- a/lib/galaxy/tool_util/linters/xml_order.py +++ b/lib/galaxy/tool_util/linters/xml_order.py @@ -4,6 +4,8 @@ https://github.com/galaxy-iuc/standards. """ +from typing import Optional + # https://github.com/galaxy-iuc/standards # https://github.com/galaxy-iuc/standards/pull/7/files TAG_ORDER = [ @@ -54,7 +56,7 @@ def lint_xml_order(tool_xml, lint_ctx): tag_ordering = TAG_ORDER last_tag = None - last_key = None + last_key: Optional[int] = None for elem in tool_root: tag = elem.tag if tag in tag_ordering: diff --git a/lib/galaxy/tool_util/parser/output_actions.py b/lib/galaxy/tool_util/parser/output_actions.py index add078262dbd..13ae26266296 100644 --- a/lib/galaxy/tool_util/parser/output_actions.py +++ b/lib/galaxy/tool_util/parser/output_actions.py @@ -324,7 +324,7 @@ def apply_action(self, output_dataset, other_values) -> None: else: # fallback when Cheetah not available, equivalent to how this was handled prior 23.0 # definitely not needed for CWL tool parsing - log.warning("Cheetah not installed, falling back to legacy 'apply_action' behavior.") + log.warning("Cheetah not installed, falling back to legacy 'apply_action' behavior.") # type: ignore[unreachable] value = self.default if value is not None: setattr(output_dataset.metadata, self.name, value) diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index 5ada4549c5aa..2c7b6985a4e3 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -761,7 +761,7 @@ def get_tool(self, tool_id, tool_version=None, get_all_versions=False, exact=Fal def has_tool(self, tool_id: str, tool_version: Optional[str] = None, exact: bool = False): return self.get_tool(tool_id, tool_version=tool_version, exact=exact) is not None - def is_missing_shed_tool(self, tool_id: str) -> bool: + def is_missing_shed_tool(self, tool_id: Optional[str]) -> bool: """Confirm that the tool ID does reference a shed tool and is not installed.""" if tool_id is None: # This is not a tool ID. diff --git a/lib/galaxy/tool_util/verify/__init__.py b/lib/galaxy/tool_util/verify/__init__.py index 83e6a2c3fd23..9fff6cafac9b 100644 --- a/lib/galaxy/tool_util/verify/__init__.py +++ b/lib/galaxy/tool_util/verify/__init__.py @@ -39,7 +39,7 @@ def verify( item_label: str, output_content: bytes, - attributes: Dict[str, Any], + attributes: Optional[Dict[str, Any]], filename: Optional[str] = None, get_filecontent: Optional[Callable[[str], bytes]] = None, get_filename: Optional[Callable[[str], str]] = None, @@ -51,6 +51,7 @@ def verify( Throw an informative assertion error if any of these tests fail. """ + attributes = attributes or {} if get_filename is None: get_filecontent_: Callable[[str], bytes] if get_filecontent is None: @@ -95,9 +96,6 @@ def get_filename(filename: str) -> str: errmsg += unicodify(err) raise AssertionError(errmsg) - if attributes is None: - attributes = {} - # expected object might be None, so don't pull unless available has_expected_object = "object" in attributes if has_expected_object: diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 00fcb610518a..ca7f30456ce0 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -271,23 +271,13 @@ def __init__(self, path: str, modtime: Optional[float] = None, lazy_hash: bool = self.hash # noqa: B018 def modtime_less_than(self, other_modtime: float): - if self._modtime is None: - # For the purposes of the tool cache, - # if we haven't seen the modtime we consider it not equal - return True return self._modtime < other_modtime def hash_equals(self, other_hash: Optional[str]): - if self._tool_hash is None or other_hash is None: - # For the purposes of the tool cache, - # if we haven't seen the hash yet we consider it not equal - return False return self.hash == other_hash @property def modtime(self) -> float: - if self._modtime is None: - self._modtime = os.path.getmtime(self.path) return self._modtime @modtime.setter diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index dc764168588b..249d8f293739 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -176,7 +176,7 @@ def str_removeprefix(s: str, prefix: str): """ if sys.version_info >= (3, 9): return s.removeprefix(prefix) - if s.startswith(prefix): + if s.startswith(prefix): # type: ignore[unreachable] return s[len(prefix) :] return s @@ -356,9 +356,9 @@ def parse_xml( if schema: schema.assertValid(tree) except OSError as e: - if e.errno is None and not os.path.exists(fname): + if e.errno is None and not os.path.exists(fname): # type: ignore[unreachable] # lxml doesn't set errno - e.errno = errno.ENOENT + e.errno = errno.ENOENT # type: ignore[unreachable] raise except etree.ParseError: log.exception("Error parsing file %s", fname) diff --git a/lib/tool_shed/test/base/twilltestcase.py b/lib/tool_shed/test/base/twilltestcase.py index 1211f3e432bb..9cfc353d695a 100644 --- a/lib/tool_shed/test/base/twilltestcase.py +++ b/lib/tool_shed/test/base/twilltestcase.py @@ -16,6 +16,7 @@ List, Optional, Tuple, + Union, ) from urllib.parse import ( quote_plus, @@ -610,7 +611,9 @@ class ShedTwillTestCase(ShedApiTestCase): """Class of FunctionalTestCase geared toward HTML interactions using the Twill library.""" requires_galaxy: bool = False - _installation_client = None + _installation_client: Optional[ + Union[StandaloneToolShedInstallationClient, GalaxyInteractorToolShedInstallationClient] + ] = None __browser: Optional[ShedBrowser] = None def setUp(self): diff --git a/mypy.ini b/mypy.ini index e9da196321af..631e0ae62362 100644 --- a/mypy.ini +++ b/mypy.ini @@ -12,6 +12,8 @@ pretty = True no_implicit_reexport = True no_implicit_optional = True strict_equality = True +warn_unreachable = True +platform = linux # green list - work on growing these please! [mypy-galaxy.util.compression_utils] From 708b487d75047613151ebf5f1c30dd51a2acd477 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:00:29 +0100 Subject: [PATCH 02/18] Fix dataype split signature --- lib/galaxy/datatypes/blast.py | 2 +- lib/galaxy/datatypes/data.py | 2 +- lib/galaxy/datatypes/molecules.py | 11 ++++++----- lib/galaxy/datatypes/msa.py | 3 ++- lib/galaxy/datatypes/sequence.py | 8 ++++---- lib/galaxy/datatypes/spaln.py | 2 +- lib/galaxy/tools/__init__.py | 2 +- 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/datatypes/blast.py b/lib/galaxy/datatypes/blast.py index b660755afa7d..c19244acb28f 100644 --- a/lib/galaxy/datatypes/blast.py +++ b/lib/galaxy/datatypes/blast.py @@ -261,7 +261,7 @@ def merge(split_files: List[str], output_file: str) -> None: raise NotImplementedError("Merging BLAST databases is non-trivial (do this via makeblastdb?)") @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """Split a BLAST database (not implemented for now).""" if split_params is None: return None diff --git a/lib/galaxy/datatypes/data.py b/lib/galaxy/datatypes/data.py index e169064d89b5..2c12f640af76 100644 --- a/lib/galaxy/datatypes/data.py +++ b/lib/galaxy/datatypes/data.py @@ -1110,7 +1110,7 @@ def set_peek(self, dataset: DatasetProtocol, **kwd) -> None: dataset.blurb = "file purged from disk" @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ Split the input files by line. """ diff --git a/lib/galaxy/datatypes/molecules.py b/lib/galaxy/datatypes/molecules.py index 643c7a4a0614..89052a0531b0 100644 --- a/lib/galaxy/datatypes/molecules.py +++ b/lib/galaxy/datatypes/molecules.py @@ -5,6 +5,7 @@ Callable, Dict, List, + Optional, ) from galaxy.datatypes import metadata @@ -261,7 +262,7 @@ def set_meta(self, dataset: DatasetProtocol, overwrite: bool = True, **kwd) -> N dataset.metadata.number_of_molecules = count_special_lines(r"^\$\$\$\$$", dataset.get_file_name()) @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ Split the input files by molecule records. """ @@ -344,7 +345,7 @@ def set_meta(self, dataset: DatasetProtocol, overwrite: bool = True, **kwd) -> N dataset.metadata.number_of_molecules = count_special_lines("@MOLECULE", dataset.get_file_name()) @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ Split the input files by molecule records. """ @@ -430,7 +431,7 @@ def set_meta(self, dataset: DatasetProtocol, overwrite: bool = True, **kwd) -> N dataset.metadata.number_of_molecules = count_special_lines("^#", dataset.get_file_name(), invert=True) @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ Split the input files by fingerprint records. """ @@ -554,7 +555,7 @@ def merge(split_files: List[str], output_file: str) -> None: raise NotImplementedError("Merging Fastsearch indices is not supported.") @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """Splitting Fastsearch indices is not supported.""" if split_params is None: return None @@ -1631,7 +1632,7 @@ def sniff_prefix(self, file_prefix: FilePrefix) -> bool: return True @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ Split the input files by molecule records. """ diff --git a/lib/galaxy/datatypes/msa.py b/lib/galaxy/datatypes/msa.py index a7cc5b40353f..326213dfbaee 100644 --- a/lib/galaxy/datatypes/msa.py +++ b/lib/galaxy/datatypes/msa.py @@ -6,6 +6,7 @@ Callable, Dict, List, + Optional, ) from galaxy.datatypes.binary import Binary @@ -205,7 +206,7 @@ def set_meta(self, dataset: DatasetProtocol, overwrite: bool = True, **kwd) -> N ) @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ Split the input files by model records. diff --git a/lib/galaxy/datatypes/sequence.py b/lib/galaxy/datatypes/sequence.py index 66a3033ed008..d7fc89a30363 100644 --- a/lib/galaxy/datatypes/sequence.py +++ b/lib/galaxy/datatypes/sequence.py @@ -217,7 +217,7 @@ def get_subdir(idx): return directories @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """Split a generic sequence file (not sensible or possible, see subclasses).""" if split_params is None: return None @@ -335,7 +335,7 @@ class Alignment(data.Text): ) @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """Split a generic alignment file (not sensible or possible, see subclasses).""" if split_params is None: return None @@ -405,7 +405,7 @@ def sniff_prefix(self, file_prefix: FilePrefix) -> bool: return False @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """Split a FASTA file sequence by sequence. Note that even if split_mode="number_of_parts", the actual number of @@ -792,7 +792,7 @@ def display_data( return Sequence.display_data(self, trans, dataset, preview, filename, to_ext, **kwd) @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """ FASTQ files are split on cluster boundaries, in increments of 4 lines """ diff --git a/lib/galaxy/datatypes/spaln.py b/lib/galaxy/datatypes/spaln.py index 1a04e74001d2..c3728ef6b4ea 100644 --- a/lib/galaxy/datatypes/spaln.py +++ b/lib/galaxy/datatypes/spaln.py @@ -172,7 +172,7 @@ def merge(split_files: List[str], output_file: str) -> None: raise NotImplementedError("Merging spaln databases is not possible") @classmethod - def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Dict) -> None: + def split(cls, input_datasets: List, subdir_generator_function: Callable, split_params: Optional[Dict]) -> None: """Split a spaln database (not implemented).""" if split_params is None: return None diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 3213f10b218b..70c5dad02c11 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -789,7 +789,7 @@ def __init__( # tool_data_table_conf.xml entries exist. self.input_params: List[ToolParameter] = [] # Attributes of tools installed from Galaxy tool sheds. - self.tool_shed = None + self.tool_shed: Optional[str] = None self.repository_name = None self.repository_owner = None self.changeset_revision = None From 30699008af365d715abd5893dd5e478b6e616ed0 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:14:36 +0100 Subject: [PATCH 03/18] Drop unreachable return statement --- lib/galaxy/datatypes/text.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/galaxy/datatypes/text.py b/lib/galaxy/datatypes/text.py index e7670856665b..f62ff3ace52d 100644 --- a/lib/galaxy/datatypes/text.py +++ b/lib/galaxy/datatypes/text.py @@ -1113,7 +1113,6 @@ def _looks_like_yaml(self, file_prefix: FilePrefix) -> bool: return True except yaml.YAMLError: return False - return False @build_sniff_from_prefix From 2a8628aab0e7cc3c4a86d110dbf3e3298bd11a78 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:14:52 +0100 Subject: [PATCH 04/18] Fix _screenshot_path signature --- lib/galaxy/selenium/context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/selenium/context.py b/lib/galaxy/selenium/context.py index 6ba867e88571..85ed6b57fe37 100644 --- a/lib/galaxy/selenium/context.py +++ b/lib/galaxy/selenium/context.py @@ -41,7 +41,7 @@ def screenshot(self, label: str): return target @abstractmethod - def _screenshot_path(self, label: str, extension=".png") -> str: + def _screenshot_path(self, label: str, extension=".png") -> Optional[str]: """Path to store screenshots in.""" From 73f23477bf928f96bff10dfaab91f2c9a08b0a70 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:15:30 +0100 Subject: [PATCH 05/18] Create previse overload for _expand_here_template --- lib/galaxy/tool_util/data/__init__.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/galaxy/tool_util/data/__init__.py b/lib/galaxy/tool_util/data/__init__.py index 08ca6d82ff48..33c2e7eced12 100644 --- a/lib/galaxy/tool_util/data/__init__.py +++ b/lib/galaxy/tool_util/data/__init__.py @@ -26,6 +26,7 @@ Dict, List, Optional, + overload, Set, Tuple, Type, @@ -878,7 +879,17 @@ def to_dict(self, view: str = "collection", value_mapper: Optional[Dict[str, Cal return rval +@overload def _expand_here_template(content: str, here: Optional[str]) -> str: + ... + + +@overload +def _expand_here_template(content: None, here: Optional[str]) -> None: + ... + + +def _expand_here_template(content: Optional[str], here: Optional[str]) -> Optional[str]: if here and content: content = string.Template(content).safe_substitute({"__HERE__": here}) return content From 508c72c67971afdcce9490d1e74882a7a7f9dbbf Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:16:16 +0100 Subject: [PATCH 06/18] Ignore unreachable code, wrong tus uploader annotation --- lib/galaxy_test/api/test_tools_upload.py | 2 +- test/integration/test_job_files.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy_test/api/test_tools_upload.py b/lib/galaxy_test/api/test_tools_upload.py index ef35eecf6de7..64d7e97365b9 100644 --- a/lib/galaxy_test/api/test_tools_upload.py +++ b/lib/galaxy_test/api/test_tools_upload.py @@ -991,7 +991,7 @@ def upload_file(url, path, api_key, history_id): uploader = my_client.uploader(path, metadata=metadata) uploader.upload() assert uploader.url - return uploader.url.rsplit("/", 1)[1] + return uploader.url.rsplit("/", 1)[1] # type: ignore[unreachable] with self.dataset_populator.test_history() as history_id: session_id = upload_file( diff --git a/test/integration/test_job_files.py b/test/integration/test_job_files.py index cf43c2416813..36babdcebf6e 100644 --- a/test/integration/test_job_files.py +++ b/test/integration/test_job_files.py @@ -138,7 +138,7 @@ def test_write_with_tus(self): uploader.upload() upload_session_url = uploader.url assert upload_session_url - tus_session_id = upload_session_url.rsplit("/", 1)[1] + tus_session_id = upload_session_url.rsplit("/", 1)[1] # type: ignore[unreachable] data = {"path": path, "job_key": job_key, "session_id": tus_session_id} post_url = self._api_url(f"jobs/{job_id}/files", use_key=False) From db82b963df97e8642a520ac0a5482644c4c5336d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:33:49 +0100 Subject: [PATCH 07/18] Drop unreachable cwl parser code --- lib/galaxy/tool_util/cwl/parser.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/galaxy/tool_util/cwl/parser.py b/lib/galaxy/tool_util/cwl/parser.py index 6236eb9d8793..6f5dc91918d1 100644 --- a/lib/galaxy/tool_util/cwl/parser.py +++ b/lib/galaxy/tool_util/cwl/parser.py @@ -327,12 +327,8 @@ def _ensure_cwl_job_initialized(self): beta_relaxed_fmt_check=beta_relaxed_fmt_check, ) - args = [] + args = [RuntimeContext(job_args)] kwargs: Dict[str, str] = {} - if RuntimeContext is not None: - args.append(RuntimeContext(job_args)) - else: - kwargs = job_args self._cwl_job = next(self._tool_proxy._tool.job(self._input_dict, self._output_callback, *args, **kwargs)) self._is_command_line_job = hasattr(self._cwl_job, "command_line") From 69a0f1ca0fb78e838c4aa11d95c9160ac24734d6 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:34:16 +0100 Subject: [PATCH 08/18] Drop unreachable break --- lib/tool_shed/test/base/twilltestcase.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/tool_shed/test/base/twilltestcase.py b/lib/tool_shed/test/base/twilltestcase.py index 9cfc353d695a..968109ac0235 100644 --- a/lib/tool_shed/test/base/twilltestcase.py +++ b/lib/tool_shed/test/base/twilltestcase.py @@ -2098,7 +2098,6 @@ def _wait_for_installation(repository: galaxy_model.ToolShedRepository, refresh) "Repository installation timed out, %d seconds elapsed, repository state is %s." % (timeout_counter, repository.status) ) - break time.sleep(1) From ca5ccdc5b893f39832910b1ad5a9db51d8d252ab Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:44:14 +0100 Subject: [PATCH 09/18] Make sure tool_shed is actually set --- lib/galaxy/tools/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 70c5dad02c11..e7b038ee65fe 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1659,6 +1659,7 @@ def populate_resource_parameters(self, tool_source): def populate_tool_shed_info(self, tool_shed_repository): if tool_shed_repository: self.tool_shed = tool_shed_repository.tool_shed + assert self.tool_shed self.repository_name = tool_shed_repository.name self.repository_owner = tool_shed_repository.owner self.changeset_revision = tool_shed_repository.changeset_revision From 97f52764075561fb12984b087c41e0ab965b29c8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:46:09 +0100 Subject: [PATCH 10/18] Add warn_unreachable excludes --- mypy.ini | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/mypy.ini b/mypy.ini index 631e0ae62362..2f567e7b72a4 100644 --- a/mypy.ini +++ b/mypy.ini @@ -38,8 +38,57 @@ disallow_untyped_decorators = True no_implicit_optional = True warn_unused_ignores = True strict_equality = True +warn_unreachable = False # red list - work on reducing these please! +[mypy-galaxy.celery] +warn_unreachable = False +[mypy-galaxy.config] +warn_unreachable = False +[mypy-galaxy.datatypes.constructive_solid_geometry] +warn_unreachable = False +[mypy-galaxy.datatypes.data] +warn_unreachable = False +[mypy-galaxy.datatypes.mothur] +warn_unreachable = False +[mypy-galaxy.datatypes.phylip] +warn_unreachable = False +[mypy-galaxy.datatypes.qiime2] +warn_unreachable = False +[mypy-galaxy.managers.jobs] +warn_unreachable = False +[mypy-galaxy.tool_shed.metadata.metadata_generator] +warn_unreachable = False +[mypy-galaxy.tool_util.cwl.parser] +warn_unreachable = False +[mypy-galaxy.tool_util.cwl.cwltool_deps] +warn_unreachable = False +[mypy-galaxy.tool_util.data] +warn_unreachable = False +[mypy-galaxy.tool_util.toolbox.base] +warn_unreachable = False +[mypy-galaxy.tool_util.verify.interactor] +warn_unreachable = False +[mypy-galaxy.tools] +warn_unreachable = False +[mypy-galaxy.tools.parameters.basic] +warn_unreachable = False +[mypy-galaxy.webapps.galaxy.api.library_datasets] +warn_unreachable = False +[mypy-galaxy.webapps.galaxy.services.jobs] +warn_unreachable = False +[mypy-galaxy.workflow.run_request] +warn_unreachable = False +[mypy-galaxy_test.driver.driver_util] +warn_unreachable = False +[mypy-tool_shed.metadata.repository_metadata_manager] +warn_unreachable = False +[mypy-test.unit.app.authnz.test_custos_authnz] +warn_unreachable = False +[mypy-test.unit.app.managers.test_HistoryManager] +warn_unreachable = False +[mypy-test.unit.tool_shed/test_tool_panel_manager] +warn_unreachable = False [mypy-galaxy.util.oset] # lots of tricky code in here... check_untyped_defs = False @@ -331,6 +380,7 @@ check_untyped_defs = False [mypy-galaxy.model.tool_shed_install.mapping] check_untyped_defs = False [mypy-galaxy.model.store] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.model.orm.scripts] check_untyped_defs = False @@ -383,6 +433,7 @@ check_untyped_defs = False [mypy-tool_shed.utility_containers.utility_container_manager] check_untyped_defs = False [mypy-tool_shed.dependencies.attribute_handlers] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.tool_util.parser.cwl] check_untyped_defs = False @@ -391,6 +442,7 @@ check_untyped_defs = False [mypy-galaxy.datatypes.triples] check_untyped_defs = False [mypy-galaxy.datatypes.tabular] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.datatypes.sequence] check_untyped_defs = False @@ -407,8 +459,10 @@ check_untyped_defs = False [mypy-galaxy.datatypes.molecules] check_untyped_defs = False [mypy-galaxy.datatypes.interval] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.datatypes.genetics] +warn_unreachable = False check_untyped_defs = False [mypy-tool_shed.webapp.model] check_untyped_defs = False @@ -471,6 +525,7 @@ check_untyped_defs = False [mypy-galaxy.tools.error_reports] check_untyped_defs = False [mypy-galaxy.jobs.dynamic_tool_destination] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.jobs.runners.state_handler_factory] check_untyped_defs = False @@ -479,6 +534,7 @@ check_untyped_defs = False [mypy-galaxy.jobs.mapper] check_untyped_defs = False [mypy-galaxy.jobs.runners] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.jobs] check_untyped_defs = False @@ -549,6 +605,7 @@ check_untyped_defs = False [mypy-galaxy.webapps.galaxy.controllers.admin] check_untyped_defs = False [mypy-galaxy.webapps.base.webapp] +warn_unreachable = False check_untyped_defs = False [mypy-starlette_context.*] no_implicit_reexport = False @@ -583,8 +640,10 @@ check_untyped_defs = False [mypy-galaxy.webapps.galaxy.api.jobs] check_untyped_defs = False [mypy-galaxy.webapps.galaxy.api.item_tags] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.webapps.galaxy.api.history_contents] +warn_unreachable = False check_untyped_defs = False [mypy-galaxy.webapps.galaxy.api.genomes] check_untyped_defs = False From 26e6f3652e4b648b0652c60deed2829b2a377d6c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Jan 2024 16:54:46 +0100 Subject: [PATCH 11/18] Add remaining exlcudes --- lib/galaxy_test/api/test_workflow_extraction.py | 2 +- mypy.ini | 6 ------ test/unit/app/authnz/test_custos_authnz.py | 3 ++- test/unit/app/managers/test_HistoryManager.py | 2 +- test/unit/tool_shed/test_tool_panel_manager.py | 2 +- 5 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/galaxy_test/api/test_workflow_extraction.py b/lib/galaxy_test/api/test_workflow_extraction.py index 7b964223df19..d57554de6998 100644 --- a/lib/galaxy_test/api/test_workflow_extraction.py +++ b/lib/galaxy_test/api/test_workflow_extraction.py @@ -117,7 +117,7 @@ def test_extract_copied_mapping_from_history_reimported(self, history_id): "Mapping connection for copied collections not yet implemented in history import/export" ) - old_history_id = self.dataset_populator.new_history() + old_history_id = self.dataset_populator.new_history() # type: ignore[unreachable] hdca, job_id1, job_id2 = self.__run_random_lines_mapped_over_singleton(old_history_id) old_contents = self._history_contents(old_history_id) diff --git a/mypy.ini b/mypy.ini index 2f567e7b72a4..d10c673b7d45 100644 --- a/mypy.ini +++ b/mypy.ini @@ -83,12 +83,6 @@ warn_unreachable = False warn_unreachable = False [mypy-tool_shed.metadata.repository_metadata_manager] warn_unreachable = False -[mypy-test.unit.app.authnz.test_custos_authnz] -warn_unreachable = False -[mypy-test.unit.app.managers.test_HistoryManager] -warn_unreachable = False -[mypy-test.unit.tool_shed/test_tool_panel_manager] -warn_unreachable = False [mypy-galaxy.util.oset] # lots of tricky code in here... check_untyped_defs = False diff --git a/test/unit/app/authnz/test_custos_authnz.py b/test/unit/app/authnz/test_custos_authnz.py index 23a3964ea980..f7beb2d3351d 100644 --- a/test/unit/app/authnz/test_custos_authnz.py +++ b/test/unit/app/authnz/test_custos_authnz.py @@ -6,6 +6,7 @@ datetime, timedelta, ) +from typing import Optional from unittest import SkipTest from urllib.parse import ( parse_qs, @@ -189,7 +190,7 @@ def one_or_none(self): class Query: external_user_id = None provider = None - custos_authnz_token = None + custos_authnz_token: Optional[CustosAuthnzToken] = None def filter_by(self, email=None, external_user_id=None, provider=None, username=None): self.external_user_id = external_user_id diff --git a/test/unit/app/managers/test_HistoryManager.py b/test/unit/app/managers/test_HistoryManager.py index e9e011c3bf97..ba1710740540 100644 --- a/test/unit/app/managers/test_HistoryManager.py +++ b/test/unit/app/managers/test_HistoryManager.py @@ -267,7 +267,7 @@ def test_sharable(self): assert len(self.history_manager.get_share_assocs(item1, user=non_owner)) == 1 assert isinstance(item1.slug, str) - self.log("should be able to unshare with specific users") + self.log("should be able to unshare with specific users") # type: ignore[unreachable] share_assoc = self.history_manager.unshare_with(item1, non_owner) assert isinstance(share_assoc, model.HistoryUserShareAssociation) assert not self.history_manager.is_accessible(item1, non_owner) diff --git a/test/unit/tool_shed/test_tool_panel_manager.py b/test/unit/tool_shed/test_tool_panel_manager.py index 9217b7347c63..b07166fc0a63 100644 --- a/test/unit/tool_shed/test_tool_panel_manager.py +++ b/test/unit/tool_shed/test_tool_panel_manager.py @@ -107,7 +107,7 @@ def test_add_twice(self): # New GUID replaced old one in tool panel but both # appear in integrated tool panel. if previous_guid: - assert (f"tool_{previous_guid}") not in section.panel_items() + assert (f"tool_{previous_guid}") not in section.panel_items() # type: ignore[unreachable] assert (f"tool_{guid}") in self.toolbox._integrated_tool_panel["tid1"].panel_items() previous_guid = guid From f9e3380a48d1d7be5dbb0d0dbc2c7d25b33d7e84 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 27 Jan 2024 11:11:20 +0100 Subject: [PATCH 12/18] Drop unreachable compression extension stripping Not going to do further work on upload1, so not fixing this. --- tools/data_source/upload.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py index 704db36b7ca7..823fee7cc9bf 100644 --- a/tools/data_source/upload.py +++ b/tools/data_source/upload.py @@ -84,7 +84,6 @@ def parse_outputs(args): def add_file(dataset, registry, output_path: str) -> Dict[str, str]: ext = None - compression_type = None line_count = None link_data_only_str = dataset.get("link_data_only", "copy_files") if link_data_only_str not in ["link_to_files", "copy_files"]: @@ -150,14 +149,6 @@ def add_file(dataset, registry, output_path: str) -> Dict[str, str]: convert_spaces_to_tabs=dataset.space_to_tab, ) - # Strip compression extension from name - if ( - compression_type - and not getattr(datatype, "compressed", False) - and dataset.name.endswith("." + compression_type) - ): - dataset.name = dataset.name[: -len("." + compression_type)] - # Move dataset if link_data_only: # Never alter a file that will not be copied to Galaxy's local file store. From 5e16978e12aaa8bdfc11a5251d428ab8328038f6 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 27 Jan 2024 11:21:11 +0100 Subject: [PATCH 13/18] Fix or ignore additional unreachable code --- lib/galaxy/jobs/runners/state_handlers/_safe_eval.py | 1 - tools/filters/axt_to_lav.py | 2 +- tools/filters/gff/gff_filter_by_attribute.py | 7 +------ tools/filters/gff_to_bed_converter.py | 4 ++-- tools/next_gen_conversion/solid2fastq.py | 1 - tools/stats/filtering.py | 3 --- 6 files changed, 4 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py b/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py index e43292084e2c..02bb6374d5be 100644 --- a/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py +++ b/lib/galaxy/jobs/runners/state_handlers/_safe_eval.py @@ -1,5 +1,4 @@ from ast import ( - Module, parse, walk, ) diff --git a/tools/filters/axt_to_lav.py b/tools/filters/axt_to_lav.py index cfdb61d50467..ca4eb745c8c0 100644 --- a/tools/filters/axt_to_lav.py +++ b/tools/filters/axt_to_lav.py @@ -74,7 +74,7 @@ def main(): elif primary is None and val is None: primary = arg elif secondary is None and val is None: - secondary = arg + secondary = arg # type: ignore[unreachable] else: usage("unknown argument: %s" % arg) diff --git a/tools/filters/gff/gff_filter_by_attribute.py b/tools/filters/gff/gff_filter_by_attribute.py index 7c9cbf1b5a9a..f2acf71c1f29 100644 --- a/tools/filters/gff/gff_filter_by_attribute.py +++ b/tools/filters/gff/gff_filter_by_attribute.py @@ -10,7 +10,6 @@ import sys from ast import ( - Module, parse, walk, ) @@ -112,8 +111,6 @@ def check_simple_name(text): except SyntaxError: return False - if not isinstance(module, Module): - return False statements = module.body if not len(statements) == 1: return False @@ -171,11 +168,9 @@ def check_expression(text): except SyntaxError: return False - if not isinstance(module, Module): - return False statements = module.body if not len(statements) == 1: - return False + return False # type: ignore[unreachable] expression = statements[0] if expression.__class__.__name__ != "Expr": return False diff --git a/tools/filters/gff_to_bed_converter.py b/tools/filters/gff_to_bed_converter.py index b45a71faf31b..81cb1ee1c047 100644 --- a/tools/filters/gff_to_bed_converter.py +++ b/tools/filters/gff_to_bed_converter.py @@ -92,7 +92,7 @@ def __main__(): # Write previous transcript. if cur_transcript_id: # Write BED entry. - out.write( + out.write( # type: ignore[unreachable] get_bed_line( cur_transcript_chrome, cur_transcript_id, @@ -120,7 +120,7 @@ def __main__(): # Write previous transcript. if cur_transcript_id: # Write BED entry. - out.write( + out.write( # type: ignore[unreachable] get_bed_line( cur_transcript_chrome, cur_transcript_id, cur_transcript_strand, cur_transcripts_blocks ) diff --git a/tools/next_gen_conversion/solid2fastq.py b/tools/next_gen_conversion/solid2fastq.py index 7a57ccd6162d..d763375983e4 100644 --- a/tools/next_gen_conversion/solid2fastq.py +++ b/tools/next_gen_conversion/solid2fastq.py @@ -25,7 +25,6 @@ def solid2sanger(quality_string, min_qual=0): qv = "0" if int(qv) < min_qual: return False - break sanger += chr(int(qv) + 33) except Exception: pass diff --git a/tools/stats/filtering.py b/tools/stats/filtering.py index 125c85ac026d..5294103dd181 100644 --- a/tools/stats/filtering.py +++ b/tools/stats/filtering.py @@ -10,7 +10,6 @@ import re import sys from ast import ( - Module, parse, walk, ) @@ -139,8 +138,6 @@ def check_expression(text): except SyntaxError: return False - if not isinstance(module, Module): - return False statements = module.body if not len(statements) == 1: return False From de0caee3648e704238910641b218a43892ddb25f Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Feb 2024 10:20:14 +0100 Subject: [PATCH 14/18] Ensure cwltool is installed and fail explicitly --- lib/galaxy/tool_util/cwl/parser.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/galaxy/tool_util/cwl/parser.py b/lib/galaxy/tool_util/cwl/parser.py index 6f5dc91918d1..674b3abcc357 100644 --- a/lib/galaxy/tool_util/cwl/parser.py +++ b/lib/galaxy/tool_util/cwl/parser.py @@ -293,6 +293,7 @@ class ExpressionToolProxy(CommandLineToolProxy): class JobProxy: def __init__(self, tool_proxy, input_dict, output_dict, job_directory): + assert RuntimeContext is not None, "cwltool is not installed, cannot run CWL jobs" self._tool_proxy = tool_proxy self._input_dict = input_dict self._output_dict = output_dict From 7ca6fff42c6bff5e862c769b00479f88ea41f963 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Feb 2024 10:25:24 +0100 Subject: [PATCH 15/18] Drop attributes is not None check --- lib/galaxy/tool_util/verify/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/verify/__init__.py b/lib/galaxy/tool_util/verify/__init__.py index 9fff6cafac9b..c989355d8bce 100644 --- a/lib/galaxy/tool_util/verify/__init__.py +++ b/lib/galaxy/tool_util/verify/__init__.py @@ -68,7 +68,7 @@ def get_filename(filename: str) -> str: # Check assertions... assertions = attributes.get("assert_list", None) - if attributes is not None and assertions is not None: + if assertions is not None: try: verify_assertions(output_content, attributes["assert_list"], attributes.get("decompress", False)) except AssertionError as err: From 67cc9a9f169eef57f2bd42af06614c32e065412a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Feb 2024 10:34:03 +0100 Subject: [PATCH 16/18] Fix type annotation for xml_to_string --- lib/galaxy/tool_util/data/__init__.py | 6 ++---- lib/galaxy/util/__init__.py | 9 ++++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tool_util/data/__init__.py b/lib/galaxy/tool_util/data/__init__.py index 33c2e7eced12..655597b39f39 100644 --- a/lib/galaxy/tool_util/data/__init__.py +++ b/lib/galaxy/tool_util/data/__init__.py @@ -880,13 +880,11 @@ def to_dict(self, view: str = "collection", value_mapper: Optional[Dict[str, Cal @overload -def _expand_here_template(content: str, here: Optional[str]) -> str: - ... +def _expand_here_template(content: str, here: Optional[str]) -> str: ... @overload -def _expand_here_template(content: None, here: Optional[str]) -> None: - ... +def _expand_here_template(content: None, here: Optional[str]) -> None: ... def _expand_here_template(content: Optional[str], here: Optional[str]) -> Optional[str]: diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index 249d8f293739..6949dbafbb71 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -391,15 +391,14 @@ def parse_xml_string_to_etree(xml_string: str, strip_whitespace: bool = True) -> return ElementTree(parse_xml_string(xml_string=xml_string, strip_whitespace=strip_whitespace)) -def xml_to_string(elem: Element, pretty: bool = False) -> str: +def xml_to_string(elem: Optional[Element], pretty: bool = False) -> str: """ Returns a string from an xml tree. """ + if elem is None: + return "" try: - if elem is not None: - xml_str = etree.tostring(elem, encoding="unicode") - else: - xml_str = "" + xml_str = etree.tostring(elem, encoding="unicode") except TypeError as e: # we assume this is a comment if hasattr(elem, "text"): From b757281b38ffb25b1c3b18ed48d2ef719ce165c3 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Feb 2024 11:45:39 +0100 Subject: [PATCH 17/18] Update pre-commit config --- .pre-commit-config.yaml.sample | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml.sample b/.pre-commit-config.yaml.sample index b441facc5dec..6c98887c9ce0 100644 --- a/.pre-commit-config.yaml.sample +++ b/.pre-commit-config.yaml.sample @@ -1,10 +1,10 @@ repos: - repo: https://github.com/psf/black - rev: 22.10.0 + rev: 24.1.1 hooks: - id: black - repo: https://github.com/pycqa/flake8 - rev: 5.0.4 + rev: 7.0.0 hooks: - id: flake8 - repo: https://github.com/pre-commit/mirrors-prettier @@ -16,7 +16,7 @@ repos: additional_dependencies: - prettier@2.8.8 # Workaround. See https://github.com/pre-commit/mirrors-prettier/issues/29 - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.3.0 # Use the ref you want to point at + rev: v4.5.0 # Use the ref you want to point at hooks: - id: trailing-whitespace - id: check-merge-conflict @@ -30,7 +30,7 @@ repos: - id: shell-lint args: [--format=json] - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.27.3 + rev: 0.27.4 hooks: - id: check-github-workflows - repo: local From d092e02ec2aeb3966c08f0fae63e8fece41de65c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 1 Feb 2024 12:48:27 +0100 Subject: [PATCH 18/18] Mark tool_id required in is_missing_shed_tool method --- lib/galaxy/tool_util/toolbox/base.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index 2c7b6985a4e3..f70d2978b4f2 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -761,11 +761,8 @@ def get_tool(self, tool_id, tool_version=None, get_all_versions=False, exact=Fal def has_tool(self, tool_id: str, tool_version: Optional[str] = None, exact: bool = False): return self.get_tool(tool_id, tool_version=tool_version, exact=exact) is not None - def is_missing_shed_tool(self, tool_id: Optional[str]) -> bool: + def is_missing_shed_tool(self, tool_id: str) -> bool: """Confirm that the tool ID does reference a shed tool and is not installed.""" - if tool_id is None: - # This is not a tool ID. - return False if "repos" not in tool_id: # This is not a shed tool. return False