From a4926ee16cf3d6eca935410be80cb70bd740ac19 Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Wed, 10 Nov 2021 14:00:51 +0100
Subject: [PATCH 01/14] improve typing of
 lib/galaxy/tools/parameters/grouping.py

---
 lib/galaxy/tools/parameters/grouping.py | 68 ++++++++++++++++++++++---
 lib/galaxy/tools/test.py                |  3 ++
 setup.cfg                               |  2 -
 3 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/lib/galaxy/tools/parameters/grouping.py b/lib/galaxy/tools/parameters/grouping.py
index 483f361c8032..0774cd4e14bb 100644
--- a/lib/galaxy/tools/parameters/grouping.py
+++ b/lib/galaxy/tools/parameters/grouping.py
@@ -5,8 +5,9 @@
 import logging
 import os
 import unicodedata
+from typing import Any, Dict, List, Optional, TYPE_CHECKING
 
-from galaxy.datatypes import sniff
+from galaxy.datatypes import data, sniff
 from galaxy.exceptions import (
     AdminRequiredException,
     ConfigDoesNotAllowException,
@@ -21,6 +22,9 @@
 from galaxy.util.dictifiable import Dictifiable
 from galaxy.util.expressions import ExpressionContext
 
+if TYPE_CHECKING:
+    from galaxy.tools.parameter.basic import ToolParameter
+
 log = logging.getLogger(__name__)
 URI_PREFIXES = [f"{x}://" for x in ["http", "https", "ftp", "file", "gxfiles", "gximport", "gxuserimport", "gxftp"]]
 
@@ -91,6 +95,8 @@ def label(self):
         return f"Repeat ({self.title})"
 
     def value_to_basic(self, value, app, use_security=False):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for d in value:
             rval_dict = {}
@@ -104,6 +110,8 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         try:
             for i, d in enumerate(value):
@@ -127,6 +135,8 @@ def value_from_basic(self, value, app, ignore_errors=False):
         return rval
 
     def get_initial_value(self, trans, context):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for i in range(self.default):
             rval_dict = {'__index__': i}
@@ -136,6 +146,8 @@ def get_initial_value(self, trans, context):
         return rval
 
     def to_dict(self, trans):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         repeat_dict = super().to_dict(trans)
 
         def input_to_dict(input):
@@ -165,6 +177,8 @@ def label(self):
         return f"Section ({self.title})"
 
     def value_to_basic(self, value, app, use_security=False):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = {}
         for input in self.inputs.values():
             if input.name in value:  # parameter might be absent in unverified workflow
@@ -172,6 +186,8 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = {}
         try:
             for input in self.inputs.values():
@@ -183,13 +199,17 @@ def value_from_basic(self, value, app, ignore_errors=False):
         return rval
 
     def get_initial_value(self, trans, context):
-        rval = {}
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
+        rval: Dict[str, Any] = {}
         child_context = ExpressionContext(rval, context)
         for child_input in self.inputs.values():
             rval[child_input.name] = child_input.get_initial_value(trans, child_context)
         return rval
 
     def to_dict(self, trans):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         section_dict = super().to_dict(trans)
 
         def input_to_dict(input):
@@ -199,6 +219,25 @@ def input_to_dict(input):
         return section_dict
 
 
+class Dataset(Bunch):
+    type: str
+    file_type: str
+    dbkey: str
+    datatype: data.Data
+    warnings: List[str]
+    metadata: Dict[str, str]
+    composite_files: Dict[str, Optional[str]]
+    uuid: Optional[str]
+    tag_using_filenames: Optional[str]
+    tags: Optional[str]
+    name: str
+    primary_file: str
+    to_posix_lines: bool
+    auto_decompress: bool
+    ext: str
+    space_to_tab: bool
+
+
 class UploadDataset(Group):
     type = "upload_dataset"
 
@@ -283,6 +322,8 @@ def title_by_index(self, trans, index, context):
         return None
 
     def value_to_basic(self, value, app, use_security=False):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for d in value:
             rval_dict = {}
@@ -296,6 +337,8 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for i, d in enumerate(value):
             try:
@@ -324,6 +367,8 @@ def get_file_count(self, trans, context):
             return int(file_count)
 
     def get_initial_value(self, trans, context):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         file_count = self.get_file_count(trans, context)
         rval = []
         for i in range(file_count):
@@ -367,8 +412,7 @@ def start_of_url(content):
                     return looks_like_url
 
                 if start_of_url(url_paste):
-                    url_paste = url_paste.replace('\r', '').split('\n')
-                    for line in url_paste:
+                    for line in url_paste.replace("\r", "").split("\n"):
                         line = line.strip()
                         if line:
                             if not start_of_url(line):
@@ -560,7 +604,7 @@ def get_filenames(context):
         if d_type.composite_type is not None or force_composite:
             # handle uploading of composite datatypes
             # Only one Dataset can be created
-            dataset = Bunch()
+            dataset = Dataset()
             dataset.type = 'composite'
             dataset.file_type = file_type
             dataset.dbkey = dbkey
@@ -646,7 +690,7 @@ class Conditional(Group):
 
     def __init__(self):
         Group.__init__(self)
-        self.test_param = None
+        self.test_param: Optional["ToolParameter"] = None
         self.cases = []
         self.value_ref = None
         self.value_ref_in_group = True  # When our test_param is not part of the conditional Group, this is False
@@ -656,6 +700,8 @@ def label(self):
         return f"Conditional ({self.name})"
 
     def get_current_case(self, value):
+        if not self.test_param:
+            raise Exception("Must set 'test_param' attribute to use.")
         # Convert value to user representation
         str_value = self.test_param.to_param_dict_string(value)
         # Find the matching case
@@ -665,6 +711,8 @@ def get_current_case(self, value):
         raise ValueError("No case matched value:", self.name, str_value)
 
     def value_to_basic(self, value, app, use_security=False):
+        if not self.test_param:
+            raise Exception("Must set 'test_param' attribute to use.")
         rval = dict()
         rval[self.test_param.name] = self.test_param.value_to_basic(value[self.test_param.name], app)
         current_case = rval['__current_case__'] = self.get_current_case(value[self.test_param.name])
@@ -674,6 +722,8 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
+        if not self.test_param:
+            raise Exception("Must set 'test_param' attribute to use.")
         rval = dict()
         try:
             rval[self.test_param.name] = self.test_param.value_from_basic(value.get(self.test_param.name), app, ignore_errors)
@@ -691,6 +741,8 @@ def value_from_basic(self, value, app, ignore_errors=False):
         return rval
 
     def get_initial_value(self, trans, context):
+        if not self.test_param:
+            raise Exception("Must set 'test_param' attribute to use.")
         # State for a conditional is a plain dictionary.
         rval = {}
         # Get the default value for the 'test element' and use it
@@ -708,6 +760,8 @@ def get_initial_value(self, trans, context):
         return rval
 
     def to_dict(self, trans):
+        if not self.test_param:
+            raise Exception("Must set 'test_param' attribute to use.")
         cond_dict = super().to_dict(trans)
 
         def nested_to_dict(input):
@@ -726,6 +780,8 @@ def __init__(self):
         self.inputs = None
 
     def to_dict(self, trans):
+        if not self.inputs:
+            raise Exception("Must set 'inputs' attribute to use.")
         when_dict = super().to_dict()
 
         def input_to_dict(input):
diff --git a/lib/galaxy/tools/test.py b/lib/galaxy/tools/test.py
index 8090a9c12e78..6c5579208bfd 100644
--- a/lib/galaxy/tools/test.py
+++ b/lib/galaxy/tools/test.py
@@ -90,6 +90,7 @@ def _process_raw_inputs(tool, tool_inputs, raw_inputs, required_files, required_
     for value in tool_inputs.values():
         if isinstance(value, galaxy.tools.parameters.grouping.Conditional):
             cond_context = ParamContext(name=value.name, parent_context=parent_context)
+            assert value.test_param
             case_context = ParamContext(name=value.test_param.name, parent_context=cond_context)
             raw_input_dict = case_context.extract_value(raw_inputs)
             case_value = raw_input_dict["value"] if raw_input_dict else None
@@ -112,6 +113,7 @@ def _process_raw_inputs(tool, tool_inputs, raw_inputs, required_files, required_
                     expanded_inputs[case_context.for_state()] = processed_value
         elif isinstance(value, galaxy.tools.parameters.grouping.Section):
             context = ParamContext(name=value.name, parent_context=parent_context)
+            assert value.inputs
             for r_value in value.inputs.values():
                 expanded_input = _process_raw_inputs(tool, {context.for_state(): r_value}, raw_inputs, required_files, required_data_tables, required_loc_files, parent_context=context)
                 if expanded_input:
@@ -121,6 +123,7 @@ def _process_raw_inputs(tool, tool_inputs, raw_inputs, required_files, required_
             while True:
                 context = ParamContext(name=value.name, index=repeat_index, parent_context=parent_context)
                 updated = False
+                assert value.inputs
                 for r_value in value.inputs.values():
                     expanded_input = _process_raw_inputs(tool, {context.for_state(): r_value}, raw_inputs, required_files, required_data_tables, required_loc_files, parent_context=context)
                     if expanded_input:
diff --git a/setup.cfg b/setup.cfg
index 94bde1944145..a81e9fbff062 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -337,8 +337,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.tools.repositories]
 check_untyped_defs = False
-[mypy-galaxy.tools.parameters.grouping]
-check_untyped_defs = False
 [mypy-galaxy.tool_util.verify.interactor]
 check_untyped_defs = False
 [mypy-galaxy.tool_shed.tool_shed_registry]

From 960a5a6f54941dfc527c8f2940172ae2e73c970f Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Wed, 10 Nov 2021 16:14:58 +0100
Subject: [PATCH 02/14] improve typing of lib/galaxy/tools/parameters/basic.py

---
 lib/galaxy/tools/parameters/basic.py | 34 +++++++++++++++++-----------
 setup.cfg                            |  2 --
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py
index 3425f4d18282..60aa617b4d9e 100644
--- a/lib/galaxy/tools/parameters/basic.py
+++ b/lib/galaxy/tools/parameters/basic.py
@@ -8,6 +8,7 @@
 import os
 import os.path
 import re
+from typing import Any, Dict, List, Tuple, Union
 
 from webob.compat import cgi_FieldStorage
 
@@ -428,7 +429,7 @@ def to_python(self, value, app):
             raise err
 
     def get_initial_value(self, trans, other_values):
-        if self.value not in {None, ''}:
+        if self.value is not None and self.value != "":
             return int(self.value)
         else:
             return None
@@ -500,6 +501,8 @@ def to_python(self, value, app):
             raise err
 
     def get_initial_value(self, trans, other_values):
+        if self.value is None:
+            return None
         try:
             return float(self.value)
         except Exception:
@@ -686,7 +689,7 @@ def to_json(self, value, app, use_security):
     def to_python(self, value, app, validate=False):
         if not isinstance(value, list):
             value = [value]
-        lst = []
+        lst: List[str] = []
         for val in value:
             if val in [None, '']:
                 lst = []
@@ -997,12 +1000,14 @@ def get_initial_value(self, trans, other_values):
             if not self.optional and not self.multiple and options:
                 # Nothing selected, but not optional and not a multiple select, with some values,
                 # so we have to default to something (the HTML form will anyway)
-                value = options[0][1]
+                value2 = options[0][1]
             else:
-                value = None
+                value2 = None
         elif len(value) == 1 or not self.multiple:
-            value = value[0]
-        return value
+            value2 = value[0]
+        else:
+            value2 = value
+        return value2
 
     def to_text(self, value):
         if not isinstance(value, list):
@@ -1334,7 +1339,7 @@ def get_options(self, trans, other_values):
         """
         Show column labels rather than c1..cn if use_header_names=True
         """
-        options = []
+        options: List[Tuple[str, Union[str, Tuple[str, str]], bool]] = []
         if self.usecolnames:  # read first row - assume is a header with metadata useful for making good choices
             dataset = other_values.get(self.data_ref, None)
             try:
@@ -1472,7 +1477,7 @@ def recurse_option_elems(cur_options, option_elems):
         if self.dynamic_options:
             self.is_dynamic = True
         self.options = []
-        self.filtered = {}
+        self.filtered: Dict[str, Any] = {}
         if elem.find('filter'):
             self.is_dynamic = True
             for filter in elem.findall('filter'):
@@ -1524,7 +1529,7 @@ def recurse_options(legal_values, options):
             for option in options:
                 legal_values.append(option['value'])
                 recurse_options(legal_values, option['options'])
-        legal_values = []
+        legal_values: List[str] = []
         recurse_options(legal_values, self.get_options(trans=trans, other_values=other_values))
         return legal_values
 
@@ -1574,7 +1579,8 @@ def recurse_option(option_list, option):
                 else:
                     for opt in option['options']:
                         recurse_option(option_list, opt)
-            rval = []
+
+            rval: List[str] = []
             recurse_option(rval, get_base_option(value, self.get_options(other_values=other_values)))
             return rval or [value]
 
@@ -1607,10 +1613,10 @@ def recurse_options(initial_values, options):
         options = self.get_options(trans=trans, other_values=other_values)
         if not options:
             return None
-        initial_values = []
+        initial_values: List[str] = []
         recurse_options(initial_values, options)
         if len(initial_values) == 0:
-            initial_values = None
+            return None
         return initial_values
 
     def to_text(self, value):
@@ -1662,6 +1668,8 @@ def to_dict(self, trans, other_values=None):
 
 class BaseDataToolParameter(ToolParameter):
 
+    multiple: bool
+
     def __init__(self, tool, input_source, trans):
         super().__init__(tool, input_source)
         self.min = input_source.get('min')
@@ -2325,7 +2333,7 @@ def to_param_dict_string(self, value, other_values=None):
     def to_json(self, value, app, use_security):
         if not isinstance(value, list):
             value = [value]
-        lst = []
+        lst: List[Dict[str, str]] = []
         for item in value:
             lda_id = lda_name = None
             if isinstance(item, app.model.LibraryDatasetDatasetAssociation):
diff --git a/setup.cfg b/setup.cfg
index a81e9fbff062..99f300c7dde4 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -511,8 +511,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.tools.parameters.dynamic_options]
 check_untyped_defs = False
-[mypy-galaxy.tools.parameters.basic]
-check_untyped_defs = False
 [mypy-galaxy.tools.parameters]
 check_untyped_defs = False
 [mypy-galaxy.tools.bundled.data_source.upload]

From 069c604aa822381ae8b122da3fefb30e74372d24 Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Wed, 10 Nov 2021 16:36:09 +0100
Subject: [PATCH 03/14] improve typing of lib/galaxy/tools/execute.py

---
 lib/galaxy/tools/execute.py | 22 +++++++++++++++++-----
 setup.cfg                   |  2 --
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/lib/galaxy/tools/execute.py b/lib/galaxy/tools/execute.py
index d45419b7535a..69eb89ae8048 100644
--- a/lib/galaxy/tools/execute.py
+++ b/lib/galaxy/tools/execute.py
@@ -5,6 +5,8 @@
 """
 import collections
 import logging
+from abc import abstractmethod
+from typing import Dict, List
 
 from boltons.iterutils import remap
 
@@ -44,7 +46,9 @@ def execute(trans, tool, mapping_params, history, rerun_remap_job_id=None, colle
     )
 
     if invocation_step is None:
-        execution_tracker = ToolExecutionTracker(trans, tool, mapping_params, collection_info, completed_jobs=completed_jobs)
+        execution_tracker: ExecutionTracker = ToolExecutionTracker(
+            trans, tool, mapping_params, collection_info, completed_jobs=completed_jobs
+        )
     else:
         execution_tracker = WorkflowStepExecutionTracker(trans, tool, mapping_params, collection_info, invocation_step, completed_jobs=completed_jobs)
     execution_cache = ToolExecutionCache(trans)
@@ -100,7 +104,9 @@ def execute_single_job(execution_slice, completed_job):
     jobs_executed = 0
     has_remaining_jobs = False
     execution_slice = None
-    job_datasets = {}  # job: list of dataset instances created by job
+    job_datasets: Dict[
+        str, List[model.DatasetInstance]
+    ] = {}  # job: list of dataset instances created by job
 
     for i, execution_slice in enumerate(execution_tracker.new_execution_slices()):
         if max_num_jobs is not None and jobs_executed >= max_num_jobs:
@@ -122,10 +128,12 @@ def execute_single_job(execution_slice, completed_job):
     trans.sa_session.flush()
 
     tool_id = tool.id
-    for job in execution_tracker.successful_jobs:
+    for job2 in execution_tracker.successful_jobs:
         # Put the job in the queue if tracking in memory
-        tool.app.job_manager.enqueue(job, tool=tool, flush=False)
-        trans.log_event(f"Added job to the job queue, id: {str(job.id)}", tool_id=tool_id)
+        tool.app.job_manager.enqueue(job2, tool=tool, flush=False)
+        trans.log_event(
+            f"Added job to the job queue, id: {str(job2.id)}", tool_id=tool_id
+        )
     trans.sa_session.flush()
 
     if has_remaining_jobs:
@@ -409,6 +417,10 @@ def record_success(self, execution_slice, job, outputs):
             job_assoc.job = job
             self.trans.sa_session.add(job_assoc)
 
+    @abstractmethod
+    def new_collection_execution_slices(self):
+        pass
+
 
 # Seperate these because workflows need to track their jobs belong to the invocation
 # in the database immediately and they can be recovered.
diff --git a/setup.cfg b/setup.cfg
index 99f300c7dde4..3e0a5aa6beb5 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -557,8 +557,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.managers.ratable]
 check_untyped_defs = False
-[mypy-galaxy.tools.execute]
-check_untyped_defs = False
 [mypy-galaxy.tools.actions.upload]
 check_untyped_defs = False
 [mypy-galaxy.tools.actions.model_operations]

From ee6eeb6fe60938db36e8043718f9fa8220653012 Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Wed, 10 Nov 2021 16:43:59 +0100
Subject: [PATCH 04/14] improve typing of lib/galaxy/tools/evaluation.py

---
 lib/galaxy/tools/evaluation.py | 20 ++++++++++++--------
 setup.cfg                      |  2 --
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py
index f9e07c350384..7479448a6d1c 100644
--- a/lib/galaxy/tools/evaluation.py
+++ b/lib/galaxy/tools/evaluation.py
@@ -3,13 +3,14 @@
 import os
 import shlex
 import tempfile
+from typing import Any, cast, Dict, List
 
 from galaxy import model
 from galaxy.files import ProvidesUserFileSourcesUserContext
 from galaxy.job_execution.setup import ensure_configs_directory
 from galaxy.model.none_like import NoneDataset
 from galaxy.security.object_wrapper import wrap_with_safe_string
-from galaxy.tools import global_tool_errors
+from galaxy.tools import global_tool_errors, Tool
 from galaxy.tools.parameters import (
     visit_input_values,
     wrapped_json,
@@ -118,7 +119,7 @@ def build_param_dict(self, incoming, input_datasets, output_datasets, output_col
         compute_environment = self.compute_environment
         job_working_directory = compute_environment.working_directory()
 
-        param_dict = dict()
+        param_dict: Dict[str, Any] = {}
 
         def input():
             raise SyntaxError("Unbound variable input.")  # Don't let $input hang Python evaluation process.
@@ -211,11 +212,14 @@ def wrap_input(input_values, input):
                     else:
                         # Trick wrapper into using target conv ext (when
                         # None) without actually being a tool parameter
-                        input_values[conversion_name] = \
-                            DatasetFilenameWrapper(converted_dataset,
-                                                   datatypes_registry=self.app.datatypes_registry,
-                                                   tool=Bunch(conversion_name=Bunch(extensions=conv_ext)),
-                                                   name=conversion_name)
+                        input_values[conversion_name] = DatasetFilenameWrapper(
+                            converted_dataset,
+                            datatypes_registry=self.app.datatypes_registry,
+                            tool=cast(
+                                Tool, Bunch(conversion_name=Bunch(extensions=conv_ext))
+                            ),
+                            name=conversion_name,
+                        )
                 # Wrap actual input dataset
                 dataset = input_values[input.name]
                 wrapper_kwds = dict(
@@ -439,7 +443,7 @@ def build(self):
         config templates corresponding to this tool with these inputs on this
         compute environment.
         """
-        self.extra_filenames = []
+        self.extra_filenames: List[str] = []
         self.command_line = None
 
         try:
diff --git a/setup.cfg b/setup.cfg
index 3e0a5aa6beb5..b7a7e7c14b77 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -581,8 +581,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.jobs.mapper]
 check_untyped_defs = False
-[mypy-galaxy.tools.evaluation]
-check_untyped_defs = False
 [mypy-galaxy.jobs.runners]
 check_untyped_defs = False
 [mypy-galaxy.jobs]

From 6921c9cc0519f1841962459e4504264404af4cef Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Wed, 10 Nov 2021 17:02:36 +0100
Subject: [PATCH 05/14] improve typing of lib/galaxy/tools/actions/__init__.py

---
 lib/galaxy/tools/actions/__init__.py | 38 ++++++++++++++++++++--------
 setup.cfg                            |  2 --
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py
index 1b10fb204817..b24f0eff6fad 100644
--- a/lib/galaxy/tools/actions/__init__.py
+++ b/lib/galaxy/tools/actions/__init__.py
@@ -2,8 +2,9 @@
 import logging
 import os
 import re
+from abc import abstractmethod
 from json import dumps
-
+from typing import Any, cast, Dict, List, Set, Union
 
 from galaxy import model
 from galaxy.exceptions import ItemAccessibilityException
@@ -51,12 +52,12 @@ class ToolAction:
     been converted and validated).
     """
 
-    def execute(self, tool, trans, incoming=None, set_output_hid=True):
-        incoming = incoming or {}
-        raise TypeError("Abstract method")
+    @abstractmethod
+    def execute(self, tool, trans, incoming=None, set_output_hid=True, **kwargs):
+        pass
 
 
-class DefaultToolAction:
+class DefaultToolAction(ToolAction):
     """Default tool action is to run an external command"""
     produces_real_jobs = True
 
@@ -69,7 +70,7 @@ def _collect_input_datasets(self, tool, param_values, trans, history, current_us
         if current_user_roles is None:
             current_user_roles = trans.get_current_user_roles()
         input_datasets = {}
-        all_permissions = {}
+        all_permissions: Dict[str, Set[str]] = {}
 
         def record_permission(action, role_id):
             if action not in all_permissions:
@@ -211,7 +212,7 @@ def append_to_key(the_dict, key, value):
                 the_dict[key] = []
             the_dict[key].append(value)
 
-        input_dataset_collections = dict()
+        input_dataset_collections: Dict[str, str] = {}
 
         def visitor(input, value, prefix, parent=None, prefixed_name=None, **kwargs):
             if isinstance(input, DataToolParameter):
@@ -228,7 +229,7 @@ def visitor(input, value, prefix, parent=None, prefixed_name=None, **kwargs):
                         # collection with individual datasets. Database will still
                         # record collection which should be enought for workflow
                         # extraction and tool rerun.
-                        if hasattr(value, 'child_collection'):
+                        if isinstance(value, model.DatasetCollectionElement):
                             # if we are mapping a collection over a tool, we only require the child_collection
                             dataset_instances = value.child_collection.dataset_instances
                         else:
@@ -473,7 +474,10 @@ def handle_output(name, output, hidden=None):
                         # Output collection is mapped over and has already been copied from original job
                         continue
                     collections_manager = app.dataset_collection_manager
-                    element_identifiers = []
+                    element_identifiers: List[
+                        Dict[str, Union[str, List[Dict[str, Union[str, List[Any]]]]]]
+                    ] = []
+                    # mypy doesn't yet support recursive type definitions
                     known_outputs = output.known_outputs(input_collections, collections_manager.type_registry)
                     # Just to echo TODO elsewhere - this should be restructured to allow
                     # nested collections.
@@ -497,7 +501,19 @@ def handle_output(name, output, hidden=None):
                                     ))
                                 else:
                                     index = name_to_index[parent_id]
-                            current_element_identifiers = current_element_identifiers[index]["element_identifiers"]
+                            current_element_identifiers = cast(
+                                List[
+                                    Dict[
+                                        str,
+                                        Union[
+                                            str, List[Dict[str, Union[str, List[Any]]]]
+                                        ],
+                                    ]
+                                ],
+                                current_element_identifiers[index][
+                                    "element_identifiers"
+                                ],
+                            )
 
                         effective_output_name = output_part_def.effective_output_name
                         element = handle_output(effective_output_name, output_part_def.output_def, hidden=True)
@@ -722,7 +738,7 @@ def _record_inputs(self, trans, tool, job, incoming, inp_data, inp_dataset_colle
         # FIXME: Don't need all of incoming here, just the defined parameters
         #        from the tool. We need to deal with tools that pass all post
         #        parameters to the command as a special case.
-        reductions = {}
+        reductions: Dict[str, List[str]] = {}
         for name, dataset_collection_info_pairs in inp_dataset_collections.items():
             for (dataset_collection, reduced) in dataset_collection_info_pairs:
                 if reduced:
diff --git a/setup.cfg b/setup.cfg
index b7a7e7c14b77..d01fba2bc23e 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -547,8 +547,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.tools.cache]
 check_untyped_defs = False
-[mypy-galaxy.tools.actions]
-check_untyped_defs = False
 [mypy-galaxy.tool_util.deps.containers]
 check_untyped_defs = False
 [mypy-galaxy.managers.users]

From ee6b5aa1d82307a06fb223f00785eb5d69830964 Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Wed, 10 Nov 2021 19:06:46 +0100
Subject: [PATCH 06/14] improve typing of lib/galaxy/tools/__init__.py

---
 lib/galaxy/tools/__init__.py            | 302 ++++++++++++++++--------
 lib/galaxy/tools/parameters/grouping.py |  40 ++--
 setup.cfg                               |   2 -
 3 files changed, 220 insertions(+), 124 deletions(-)

diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
index d463fa379e3b..679fa44fdedc 100644
--- a/lib/galaxy/tools/__init__.py
+++ b/lib/galaxy/tools/__init__.py
@@ -11,7 +11,19 @@
 import threading
 from datetime import datetime
 from pathlib import Path
-from typing import cast, Dict, List, NamedTuple, Optional, Tuple, Type, Union
+from typing import (
+    Any,
+    cast,
+    Dict,
+    List,
+    NamedTuple,
+    Optional,
+    Set,
+    Tuple,
+    Type,
+    TYPE_CHECKING,
+    Union,
+)
 from urllib.parse import unquote_plus
 
 import packaging.version
@@ -51,7 +63,7 @@
 from galaxy.tool_util.toolbox import BaseGalaxyToolBox
 from galaxy.tool_util.toolbox.views.sources import StaticToolBoxViewSources
 from galaxy.tools import expressions
-from galaxy.tools.actions import DefaultToolAction
+from galaxy.tools.actions import DefaultToolAction, ToolAction
 from galaxy.tools.actions.data_manager import DataManagerToolAction
 from galaxy.tools.actions.data_source import DataSourceToolAction
 from galaxy.tools.actions.model_operations import ModelOperationToolAction
@@ -115,6 +127,8 @@
     MappingParameters,
 )
 
+if TYPE_CHECKING:
+    from galaxy.tools.actions.metadata import SetMetadataToolAction
 
 log = logging.getLogger(__name__)
 
@@ -442,7 +456,7 @@ def _get_tool_shed_repository(self, tool_shed, name, owner, installed_changeset_
 
     def __build_tool_version_select_field(self, tools, tool_id, set_selected):
         """Build a SelectField whose options are the ids for the received list of tools."""
-        options = []
+        options: List[Tuple[str, str]] = []
         for tool in tools:
             options.insert(0, (tool.version, tool.id))
         select_field = SelectField(name='tool_id')
@@ -505,6 +519,11 @@ def copy(self):
         return new_state
 
 
+class _Options(Bunch):
+    sanitize: str
+    refresh: str
+
+
 class Tool(Dictifiable):
     """
     Represents a computational tool that can be executed through Galaxy.
@@ -514,8 +533,11 @@ class Tool(Dictifiable):
     requires_setting_metadata = True
     produces_entry_points = False
     default_tool_action = DefaultToolAction
+    tool_action: ToolAction
     tool_type_local = False
     dict_collection_visible_keys = ['id', 'name', 'version', 'description', 'labels']
+    __help: Optional[threading.Lock]
+    __help_by_page: Union[threading.Lock, List[str]]
 
     def __init__(self, config_file, tool_source, app, guid=None, repository_id=None, tool_shed_repository=None, allow_code_files=True, dynamic=False):
         """Load a tool from the config named by `config_file`"""
@@ -535,9 +557,9 @@ def __init__(self, config_file, tool_source, app, guid=None, repository_id=None,
         self.stdio_regexes = list()
         self.inputs_by_page = list()
         self.display_by_page = list()
-        self.action = '/tool_runner/index'
-        self.target = 'galaxy_main'
-        self.method = 'post'
+        self.action: Union[str, Tuple[str, str]] = "/tool_runner/index"
+        self.target = "galaxy_main"
+        self.method = "post"
         self.labels = []
         self.check_values = True
         self.nginx_upload = False
@@ -728,8 +750,11 @@ def requires_galaxy_python_environment(self):
         else:
             unversioned_legacy_tool = self.old_id in GALAXY_LIB_TOOLS_UNVERSIONED
             versioned_legacy_tool = self.old_id in GALAXY_LIB_TOOLS_VERSIONED
-            legacy_tool = unversioned_legacy_tool or \
-                (versioned_legacy_tool and self.version_object < GALAXY_LIB_TOOLS_VERSIONED[self.old_id])
+            legacy_tool = unversioned_legacy_tool or (
+                versioned_legacy_tool
+                and self.old_id
+                and self.version_object < GALAXY_LIB_TOOLS_VERSIONED[self.old_id]
+            )
             return legacy_tool
 
     def __get_job_tool_configuration(self, job_params=None):
@@ -941,11 +966,12 @@ def parse(self, tool_source, guid=None, dynamic=False):
         self.__parse_legacy_features(tool_source)
 
         # Load any tool specific options (optional)
-        self.options = dict(
-            sanitize=tool_source.parse_sanitize(),
-            refresh=tool_source.parse_refresh(),
+        self.options = _Options(
+            **dict(
+                sanitize=tool_source.parse_sanitize(),
+                refresh=tool_source.parse_refresh(),
+            )
         )
-        self.options = Bunch(** self.options)
 
         # Read in name of galaxy.json metadata file and how to parse it.
         self.provided_metadata_file = tool_source.parse_provided_metadata_file()
@@ -1025,9 +1051,9 @@ def parse(self, tool_source, guid=None, dynamic=False):
         self.ports = tool_source.parse_interactivetool()
 
     def __parse_legacy_features(self, tool_source):
-        self.code_namespace = dict()
-        self.hook_map = {}
-        self.uihints = {}
+        self.code_namespace: Dict[str, str] = {}
+        self.hook_map: Dict[str, str] = {}
+        self.uihints: Dict[str, str] = {}
 
         if not hasattr(tool_source, 'root'):
             return
@@ -1051,7 +1077,11 @@ def __parse_legacy_features(self, tool_source):
                     compiled_code = compile(code_string, code_path, 'exec')
                     exec(compiled_code, self.code_namespace)
                 except Exception:
-                    if refactoring_tool and self.python_template_version.release[0] < 3:
+                    if (
+                        refactoring_tool
+                        and self.python_template_version
+                        and self.python_template_version.release[0] < 3
+                    ):
                         # Could be a code file that uses python 2 syntax
                         translated_code = str(refactoring_tool.refactor_string(code_string, name='auto_translated_code_file'))
                         compiled_code = compile(translated_code, f"futurized_{code_path}", 'exec')
@@ -1200,7 +1230,7 @@ def parse_inputs(self, tool_source):
         # Load parameters (optional)
         self.inputs = {}
         pages = tool_source.parse_input_pages()
-        enctypes = set()
+        enctypes: Set[str] = set()
         if pages.inputs_defined:
             if hasattr(pages, "input_elem"):
                 input_elem = pages.input_elem
@@ -1212,13 +1242,21 @@ def parse_inputs(self, tool_source):
                 # a string. The actual action needs to get url_for run to add any
                 # prefixes, and we want to avoid adding the prefix to the
                 # nginx_upload_path.
-                if self.nginx_upload and self.app.config.nginx_upload_path:
-                    if '?' in unquote_plus(self.action):
-                        raise Exception('URL parameters in a non-default tool action can not be used '
-                                        'in conjunction with nginx upload.  Please convert them to '
-                                        'hidden POST parameters')
-                    self.action = (f"{self.app.config.nginx_upload_path}?nginx_redir=",
-                                   unquote_plus(self.action))
+                if (
+                    self.nginx_upload
+                    and self.app.config.nginx_upload_path
+                    and not isinstance(self.action, tuple)
+                ):
+                    if "?" in unquote_plus(self.action):
+                        raise Exception(
+                            "URL parameters in a non-default tool action can not be used "
+                            "in conjunction with nginx upload.  Please convert them to "
+                            "hidden POST parameters"
+                        )
+                    self.action = (
+                        f"{self.app.config.nginx_upload_path}?nginx_redir=",
+                        unquote_plus(self.action),
+                    )
                 self.target = input_elem.get("target", self.target)
                 self.method = input_elem.get("method", self.method)
                 # Parse the actual parameters
@@ -1291,7 +1329,7 @@ def _parse_citations(self, tool_source):
             return []
 
         root = tool_source.root
-        citations = []
+        citations: List[str] = []
         citations_elem = root.find("citations")
         if citations_elem is None:
             return citations
@@ -1311,40 +1349,51 @@ def parse_input_elem(self, page_source, enctypes, context=None):
         groups (repeat, conditional) or param elements. Groups will be parsed
         recursively.
         """
-        rval = {}
+        rval: Dict[str, Any] = {}
         context = ExpressionContext(rval, context)
         for input_source in page_source.parse_input_sources():
             # Repeat group
             input_type = input_source.parse_input_type()
             if input_type == "repeat":
-                group = Repeat()
-                group.name = input_source.get("name")
-                group.title = input_source.get("title")
-                group.help = input_source.get("help", None)
+                group_r = Repeat()
+                group_r.name = input_source.get("name")
+                group_r.title = input_source.get("title")
+                group_r.help = input_source.get("help", None)
                 page_source = input_source.parse_nested_inputs_source()
-                group.inputs = self.parse_input_elem(page_source, enctypes, context)
-                group.default = int(input_source.get("default", 0))
-                group.min = int(input_source.get("min", 0))
+                group_r.inputs = self.parse_input_elem(page_source, enctypes, context)
+                group_r.default = int(input_source.get("default", 0))
+                group_r.min = int(input_source.get("min", 0))
                 # Use float instead of int so that 'inf' can be used for no max
-                group.max = float(input_source.get("max", "inf"))
-                assert group.min <= group.max, ValueError(f"Tool with id '{self.id}': min repeat count must be less-than-or-equal to the max.")
+                group_r.max = float(input_source.get("max", "inf"))
+                assert group_r.min <= group_r.max, ValueError(
+                    f"Tool with id '{self.id}': min repeat count must be less-than-or-equal to the max."
+                )
                 # Force default to be within min-max range
-                group.default = min(max(group.default, group.min), group.max)
-                rval[group.name] = group
+                group_r.default = cast(
+                    int, min(max(group_r.default, group_r.min), group_r.max)
+                )
+                rval[group_r.name] = group_r
             elif input_type == "conditional":
-                group = Conditional()
-                group.name = input_source.get("name")
-                group.value_ref = input_source.get('value_ref', None)
-                group.value_ref_in_group = input_source.get_bool('value_ref_in_group', True)
+                group_c = Conditional()
+                group_c.name = input_source.get("name")
+                group_c.value_ref = input_source.get("value_ref", None)
+                group_c.value_ref_in_group = input_source.get_bool(
+                    "value_ref_in_group", True
+                )
                 value_from = input_source.get("value_from", None)
                 if value_from:
                     value_from = value_from.split(':')
-                    group.value_from = locals().get(value_from[0])
-                    group.test_param = rval[group.value_ref]
-                    group.test_param.refresh_on_change = True
+                    temp_value_from = locals().get(value_from[0])
+                    group_c.test_param = rval[group_c.value_ref]
+                    group_c.test_param.refresh_on_change = True
                     for attr in value_from[1].split('.'):
-                        group.value_from = getattr(group.value_from, attr)
-                    for case_value, case_inputs in group.value_from(context, group, self).items():
+                        temp_value_from = getattr(temp_value_from, attr)
+                    group_c.value_from = temp_value_from  # type: ignore[assignment]
+                    # ^^ due to https://github.com/python/mypy/issues/2427
+                    assert group_c.value_from
+                    for case_value, case_inputs in group_c.value_from(
+                        context, group_c, self
+                    ).items():
                         case = ConditionalWhen()
                         case.value = case_value
                         if case_inputs:
@@ -1352,60 +1401,84 @@ def parse_input_elem(self, page_source, enctypes, context=None):
                             case.inputs = self.parse_input_elem(page_source, enctypes, context)
                         else:
                             case.inputs = {}
-                        group.cases.append(case)
+                        group_c.cases.append(case)
                 else:
                     # Should have one child "input" which determines the case
                     test_param_input_source = input_source.parse_test_input_source()
-                    group.test_param = self.parse_param_elem(test_param_input_source, enctypes, context)
-                    if group.test_param.optional:
+                    group_c.test_param = self.parse_param_elem(
+                        test_param_input_source, enctypes, context
+                    )
+                    if group_c.test_param.optional:
                         log.debug(f"Tool with id '{self.id}': declares a conditional test parameter as optional, this is invalid and will be ignored.")
-                        group.test_param.optional = False
-                    possible_cases = list(group.test_param.legal_values)  # store possible cases, undefined whens will have no inputs
+                        group_c.test_param.optional = False
+                    possible_cases = list(
+                        group_c.test_param.legal_values
+                    )  # store possible cases, undefined whens will have no inputs
                     # Must refresh when test_param changes
-                    group.test_param.refresh_on_change = True
+                    group_c.test_param.refresh_on_change = True
                     # And a set of possible cases
                     for (value, case_inputs_source) in input_source.parse_when_input_sources():
                         case = ConditionalWhen()
                         case.value = value
                         case.inputs = self.parse_input_elem(case_inputs_source, enctypes, context)
-                        group.cases.append(case)
+                        group_c.cases.append(case)
                         try:
                             possible_cases.remove(case.value)
                         except Exception:
-                            log.debug("Tool with id '%s': a when tag has been defined for '%s (%s) --> %s', but does not appear to be selectable." %
-                                      (self.id, group.name, group.test_param.name, case.value))
+                            log.debug(
+                                "Tool with id '%s': a when tag has been defined for '%s (%s) --> %s', but does not appear to be selectable."
+                                % (
+                                    self.id,
+                                    group_c.name,
+                                    group_c.test_param.name,
+                                    case.value,
+                                )
+                            )
                     for unspecified_case in possible_cases:
-                        log.warning("Tool with id '%s': a when tag has not been defined for '%s (%s) --> %s', assuming empty inputs." %
-                                    (self.id, group.name, group.test_param.name, unspecified_case))
+                        log.warning(
+                            "Tool with id '%s': a when tag has not been defined for '%s (%s) --> %s', assuming empty inputs."
+                            % (
+                                self.id,
+                                group_c.name,
+                                group_c.test_param.name,
+                                unspecified_case,
+                            )
+                        )
                         case = ConditionalWhen()
                         case.value = unspecified_case
                         case.inputs = {}
-                        group.cases.append(case)
-                rval[group.name] = group
+                        group_c.cases.append(case)
+                rval[group_c.name] = group_c
             elif input_type == "section":
-                group = Section()
-                group.name = input_source.get("name")
-                group.title = input_source.get("title")
-                group.help = input_source.get("help", None)
-                group.expanded = input_source.get_bool("expanded", False)
+                group_s = Section()
+                group_s.name = input_source.get("name")
+                group_s.title = input_source.get("title")
+                group_s.help = input_source.get("help", None)
+                group_s.expanded = input_source.get_bool("expanded", False)
                 page_source = input_source.parse_nested_inputs_source()
-                group.inputs = self.parse_input_elem(page_source, enctypes, context)
-                rval[group.name] = group
+                group_s.inputs = self.parse_input_elem(page_source, enctypes, context)
+                rval[group_s.name] = group_s
             elif input_type == "upload_dataset":
                 elem = input_source.elem()
-                group = UploadDataset()
-                group.name = elem.get("name")
-                group.title = elem.get("title")
-                group.file_type_name = elem.get('file_type_name', group.file_type_name)
-                group.default_file_type = elem.get('default_file_type', group.default_file_type)
-                group.metadata_ref = elem.get('metadata_ref', group.metadata_ref)
+                group_u = UploadDataset()
+                group_u.name = elem.get("name")
+                group_u.title = elem.get("title")
+                group_u.file_type_name = elem.get(
+                    "file_type_name", group_u.file_type_name
+                )
+                group_u.default_file_type = elem.get(
+                    "default_file_type", group_u.default_file_type
+                )
+                group_u.metadata_ref = elem.get("metadata_ref", group_u.metadata_ref)
                 try:
-                    rval[group.file_type_name].refresh_on_change = True
+                    rval[group_u.file_type_name].refresh_on_change = True
                 except KeyError:
                     pass
                 group_page_source = XmlPageSource(elem)
-                group.inputs = self.parse_input_elem(group_page_source, enctypes, context)
-                rval[group.name] = group
+                group_u.inputs = self.parse_input_elem(
+                    group_page_source, enctypes, context
+                )
+                rval[group_u.name] = group_u
             elif input_type == "param":
                 param = self.parse_param_elem(input_source, enctypes, context)
                 rval[param.name] = param
@@ -1501,7 +1574,7 @@ def __ensure_help(self):
     def __inititalize_help(self):
         tool_source = self.__help_source
         self.__help = None
-        self.__help_by_page = []
+        __help_by_page = []
         help_footer = ""
         help_text = tool_source.parse_help()
         if help_text is not None:
@@ -1527,20 +1600,25 @@ def __inititalize_help(self):
                 # Multiple help page case
                 if help_pages:
                     for help_page in help_pages:
-                        self.__help_by_page.append(help_page.text)
+                        __help_by_page.append(help_page.text)
                         help_footer = help_footer + help_page.tail
                 # Each page has to rendered all-together because of backreferences allowed by rst
                 try:
-                    self.__help_by_page = [Template(rst_to_html(help_header + x + help_footer),
-                                                    input_encoding='utf-8',
-                                                    default_filters=['decode.utf8'],
-                                                    encoding_errors='replace')
-                                           for x in self.__help_by_page]
+                    __help_by_page = [
+                        Template(
+                            rst_to_html(help_header + x + help_footer),
+                            input_encoding="utf-8",
+                            default_filters=["decode.utf8"],
+                            encoding_errors="replace",
+                        )
+                        for x in __help_by_page
+                    ]
                 except Exception:
                     log.exception("Exception while parsing multi-page help for tool with id '%s'", self.id)
         # Pad out help pages to match npages ... could this be done better?
-        while len(self.__help_by_page) < self.npages:
-            self.__help_by_page.append(self.__help)
+        while len(__help_by_page) < self.npages:
+            __help_by_page.append(self.__help)
+        self.__help_by_page = __help_by_page
 
     def find_output_def(self, name):
         # name is JobToOutputDatasetAssociation name.
@@ -1658,7 +1736,7 @@ def expand_incoming(self, trans, incoming, request_context, input_format='legacy
         all_params = []
         for expanded_incoming in expanded_incomings:
             params = {}
-            errors = {}
+            errors: Dict[str, str] = {}
             if self.input_translator:
                 self.input_translator.translate(expanded_incoming)
             if not self.check_values:
@@ -2058,8 +2136,8 @@ def to_archive(self):
         tool = self
         tarball_files = []
         temp_files = []
-        with open(os.path.abspath(tool.config_file)) as fh:
-            tool_xml = fh.read()
+        with open(os.path.abspath(tool.config_file)) as fh1:
+            tool_xml = fh1.read()
         # Retrieve tool help images and rewrite the tool's xml into a temporary file with the path
         # modified to be relative to the repository root.
         image_found = False
@@ -2079,9 +2157,11 @@ def to_archive(self):
                         tool_xml = tool_xml.replace('${static_path}/%s' % tarball_path, tarball_path)
         # If one or more tool help images were found, add the modified tool XML to the tarball instead of the original.
         if image_found:
-            with tempfile.NamedTemporaryFile(mode='w', suffix='.xml', delete=False) as fh:
-                new_tool_config = fh.name
-                fh.write(tool_xml)
+            with tempfile.NamedTemporaryFile(
+                mode="w", suffix=".xml", delete=False
+            ) as fh2:
+                new_tool_config = fh2.name
+                fh2.write(tool_xml)
             tool_tup = (new_tool_config, os.path.split(tool.config_file)[-1])
             temp_files.append(new_tool_config)
         else:
@@ -2139,15 +2219,19 @@ def to_archive(self):
                         if len(data_table_definitions) > 0:
                             # Put the data table definition XML in a temporary file.
                             table_definition = '<?xml version="1.0" encoding="utf-8"?>\n<tables>\n    %s</tables>'
-                            table_definition = table_definition % '\n'.join(data_table_definitions)
-                            with tempfile.NamedTemporaryFile(mode='w', delete=False) as fh:
-                                table_conf = fh.name
-                                fh.write(table_definition)
+                            table_definition = table_definition % "\n".join(
+                                data_table_definitions
+                            )
+                            with tempfile.NamedTemporaryFile(
+                                mode="w", delete=False
+                            ) as fh3:
+                                table_conf = fh3.name
+                                fh3.write(table_definition)
                             tarball_files.append((table_conf, os.path.join('tool-data', 'tool_data_table_conf.xml.sample')))
                             temp_files.append(table_conf)
         # Create the tarball.
-        with tempfile.NamedTemporaryFile(suffix='.tgz', delete=False) as fh:
-            tarball_archive = fh.name
+        with tempfile.NamedTemporaryFile(suffix=".tgz", delete=False) as fh4:
+            tarball_archive = fh4.name
         tarball = tarfile.open(name=tarball_archive, mode='w:gz')
         # Add the files from the previously generated list.
         for fspath, tarpath in tarball_files:
@@ -2263,8 +2347,8 @@ def to_json(self, trans, kwd=None, job=None, workflow_building_mode=False, histo
 
         set_dataset_matcher_factory(request_context, self)
         # create tool state
-        state_inputs = {}
-        state_errors = {}
+        state_inputs: Dict[str, str] = {}
+        state_errors: Dict[str, str] = {}
         populate_state(request_context, self.inputs, params.__dict__, state_inputs, state_errors)
 
         # create tool model
@@ -2320,7 +2404,8 @@ def populate_model(self, request_context, inputs, state_inputs, group_inputs, ot
             if input.type == 'repeat':
                 tool_dict = input.to_dict(request_context)
                 group_size = len(group_state)
-                group_cache = tool_dict['cache'] = [None] * group_size
+                tool_dict["cache"] = [None] * group_size
+                group_cache: List[List[str]] = tool_dict["cache"]
                 for i in range(group_size):
                     group_cache[i] = []
                     self.populate_model(request_context, input.inputs, group_state[i], group_cache[i], other_values)
@@ -2519,7 +2604,11 @@ def exec_before_job(self, app, inp_data, out_data, param_dict=None):
             json_params['output_data'].append(data_dict)
             if json_filename is None:
                 json_filename = file_name
-        with open(json_filename, 'w') as out:
+        if not json_filename:
+            raise Exception(
+                "Must call 'exec_before_job' with 'out_data' containing at least one entry."
+            )
+        with open(json_filename, "w") as out:
             out.write(json.dumps(json_params))
 
 
@@ -2567,7 +2656,7 @@ def exec_before_job(self, app, inp_data, out_data, param_dict=None):
         if param_dict is None:
             raise Exception("Internal error - param_dict is empty.")
 
-        job = {}
+        job: Dict[str, str] = {}
         json_wrap(self.inputs, param_dict, self.profile, job, handle_files='OBJECT')
         expression_inputs = {
             'job': job,
@@ -2670,7 +2759,11 @@ def exec_before_job(self, app, inp_data, out_data, param_dict=None):
             json_params['output_data'].append(data_dict)
             if json_filename is None:
                 json_filename = file_name
-        with open(json_filename, 'w') as out:
+        if not json_filename:
+            raise Exception(
+                "Must call 'exec_before_job' with 'out_data' containing at least one entry."
+            )
+        with open(json_filename, "w") as out:
             out.write(json.dumps(json_params))
 
 
@@ -2692,6 +2785,7 @@ class SetMetadataTool(Tool):
     """
     tool_type = 'set_metadata'
     requires_setting_metadata = False
+    tool_action: "SetMetadataToolAction"
 
     def regenerate_imported_metadata_if_needed(self, hda, history, job):
         if len(hda.metadata_file_types) > 0:
@@ -3032,7 +3126,7 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history
         new_element_structure = {}
 
         # Which inputs does the identifier appear in.
-        identifiers_map = {}
+        identifiers_map: Dict[str, List[int]] = {}
         for input_num, input_list in enumerate(input_lists):
             for dce in input_list.collection.elements:
                 element_identifier = dce.element_identifier
@@ -3072,7 +3166,7 @@ def produce_outputs(self, trans, out_data, output_collections, incoming, history
                     if dupl_actions == "keep_first" and identifier_seen:
                         continue
 
-                    if add_suffix:
+                    if add_suffix and suffix_pattern:
                         suffix = suffix_pattern.replace("#", str(copy + 1))
                         effective_identifer = f"{element_identifier}{suffix}"
                     else:
diff --git a/lib/galaxy/tools/parameters/grouping.py b/lib/galaxy/tools/parameters/grouping.py
index 0774cd4e14bb..9eff77a0365c 100644
--- a/lib/galaxy/tools/parameters/grouping.py
+++ b/lib/galaxy/tools/parameters/grouping.py
@@ -5,7 +5,7 @@
 import logging
 import os
 import unicodedata
-from typing import Any, Dict, List, Optional, TYPE_CHECKING
+from typing import Any, Callable, Dict, List, Mapping, Optional, TYPE_CHECKING
 
 from galaxy.datatypes import data, sniff
 from galaxy.exceptions import (
@@ -24,6 +24,7 @@
 
 if TYPE_CHECKING:
     from galaxy.tools.parameter.basic import ToolParameter
+    from galaxy.tools import Tool
 
 log = logging.getLogger(__name__)
 URI_PREFIXES = [f"{x}://" for x in ["http", "https", "ftp", "file", "gxfiles", "gximport", "gxuserimport", "gxftp"]]
@@ -95,7 +96,7 @@ def label(self):
         return f"Repeat ({self.title})"
 
     def value_to_basic(self, value, app, use_security=False):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for d in value:
@@ -110,7 +111,7 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         try:
@@ -135,7 +136,7 @@ def value_from_basic(self, value, app, ignore_errors=False):
         return rval
 
     def get_initial_value(self, trans, context):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for i in range(self.default):
@@ -146,7 +147,7 @@ def get_initial_value(self, trans, context):
         return rval
 
     def to_dict(self, trans):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         repeat_dict = super().to_dict(trans)
 
@@ -177,7 +178,7 @@ def label(self):
         return f"Section ({self.title})"
 
     def value_to_basic(self, value, app, use_security=False):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = {}
         for input in self.inputs.values():
@@ -186,7 +187,7 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = {}
         try:
@@ -199,7 +200,7 @@ def value_from_basic(self, value, app, ignore_errors=False):
         return rval
 
     def get_initial_value(self, trans, context):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval: Dict[str, Any] = {}
         child_context = ExpressionContext(rval, context)
@@ -208,7 +209,7 @@ def get_initial_value(self, trans, context):
         return rval
 
     def to_dict(self, trans):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         section_dict = super().to_dict(trans)
 
@@ -322,7 +323,7 @@ def title_by_index(self, trans, index, context):
         return None
 
     def value_to_basic(self, value, app, use_security=False):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for d in value:
@@ -337,7 +338,7 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         rval = []
         for i, d in enumerate(value):
@@ -367,7 +368,7 @@ def get_file_count(self, trans, context):
             return int(file_count)
 
     def get_initial_value(self, trans, context):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         file_count = self.get_file_count(trans, context)
         rval = []
@@ -687,6 +688,9 @@ def get_filenames(context):
 
 class Conditional(Group):
     type = "conditional"
+    value_from: Callable[
+        ["Conditional", ExpressionContext, "Conditional", "Tool"], Mapping[str, str]
+    ]
 
     def __init__(self):
         Group.__init__(self)
@@ -700,7 +704,7 @@ def label(self):
         return f"Conditional ({self.name})"
 
     def get_current_case(self, value):
-        if not self.test_param:
+        if self.test_param is None:
             raise Exception("Must set 'test_param' attribute to use.")
         # Convert value to user representation
         str_value = self.test_param.to_param_dict_string(value)
@@ -711,7 +715,7 @@ def get_current_case(self, value):
         raise ValueError("No case matched value:", self.name, str_value)
 
     def value_to_basic(self, value, app, use_security=False):
-        if not self.test_param:
+        if self.test_param is None:
             raise Exception("Must set 'test_param' attribute to use.")
         rval = dict()
         rval[self.test_param.name] = self.test_param.value_to_basic(value[self.test_param.name], app)
@@ -722,7 +726,7 @@ def value_to_basic(self, value, app, use_security=False):
         return rval
 
     def value_from_basic(self, value, app, ignore_errors=False):
-        if not self.test_param:
+        if self.test_param is None:
             raise Exception("Must set 'test_param' attribute to use.")
         rval = dict()
         try:
@@ -741,7 +745,7 @@ def value_from_basic(self, value, app, ignore_errors=False):
         return rval
 
     def get_initial_value(self, trans, context):
-        if not self.test_param:
+        if self.test_param is None:
             raise Exception("Must set 'test_param' attribute to use.")
         # State for a conditional is a plain dictionary.
         rval = {}
@@ -760,7 +764,7 @@ def get_initial_value(self, trans, context):
         return rval
 
     def to_dict(self, trans):
-        if not self.test_param:
+        if self.test_param is None:
             raise Exception("Must set 'test_param' attribute to use.")
         cond_dict = super().to_dict(trans)
 
@@ -780,7 +784,7 @@ def __init__(self):
         self.inputs = None
 
     def to_dict(self, trans):
-        if not self.inputs:
+        if self.inputs is None:
             raise Exception("Must set 'inputs' attribute to use.")
         when_dict = super().to_dict()
 
diff --git a/setup.cfg b/setup.cfg
index d01fba2bc23e..aaf08a64c60a 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -575,8 +575,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.queue_worker]
 check_untyped_defs = False
-[mypy-galaxy.tools]
-check_untyped_defs = False
 [mypy-galaxy.jobs.mapper]
 check_untyped_defs = False
 [mypy-galaxy.jobs.runners]

From 67717cc3e8df445e946307fd2937a666d395b6b3 Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Thu, 11 Nov 2021 17:13:45 +0100
Subject: [PATCH 07/14] remove Union with only one member

---
 lib/galaxy/workflow/modules.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py
index d00780a2e821..c6ce74207a96 100644
--- a/lib/galaxy/workflow/modules.py
+++ b/lib/galaxy/workflow/modules.py
@@ -932,8 +932,8 @@ def get_inputs(self):
             optional_cond.cases = optional_cases
 
             if param_type == "text":
-                restrict_how_source: Union[
-                    Dict[str, Union[str, List[Dict[str, Union[str, bool]]]]]
+                restrict_how_source: Dict[
+                    str, Union[str, List[Dict[str, Union[str, bool]]]]
                 ] = dict(name="how", label="Restrict Text Values?", type="select")
                 if parameter_def.get("restrictions") is not None:
                     restrict_how_value = "staticRestrictions"

From 2eb4afb59c7b01c6bf7b98721ba864265d11d2bb Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Thu, 11 Nov 2021 15:03:42 +0100
Subject: [PATCH 08/14] improve typing of
 lib/galaxy/managers/collections_util.py.

---
 lib/galaxy/managers/collections_util.py | 7 +++----
 setup.cfg                               | 2 --
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/managers/collections_util.py b/lib/galaxy/managers/collections_util.py
index afa2e991e4b4..f61a709f4e0a 100644
--- a/lib/galaxy/managers/collections_util.py
+++ b/lib/galaxy/managers/collections_util.py
@@ -111,7 +111,7 @@ def dictify_dataset_collection_instance(dataset_collection_instance, parent, sec
         encoded_history_id = security.encode_id(parent.id)
         dict_value['url'] = web.url_for('history_content_typed', history_id=encoded_history_id, id=encoded_id, type="dataset_collection")
     elif isinstance(parent, model.LibraryFolder):
-        encoded_library_id = security.encode_id(parent.library.id)
+        encoded_library_id = security.encode_id(parent.library.id)  # type: ignore
         encoded_folder_id = security.encode_id(parent.id)
         # TODO: Work in progress - this end-point is not right yet...
         dict_value['url'] = web.url_for('library_content', library_id=encoded_library_id, id=encoded_id, folder_id=encoded_folder_id)
@@ -163,10 +163,9 @@ def dictify_element_reference(element, rank_fuzzy_counts=None, recursive=True, s
             object_details["hda_ldda"] = 'hda'
             object_details["history_id"] = element_object.history_id
 
+        dictified["object"] = object_details
     else:
-        object_details = None
-
-    dictified["object"] = object_details
+        dictified["object"] = None
     return dictified
 
 
diff --git a/setup.cfg b/setup.cfg
index aaf08a64c60a..4e7c15ff49d1 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -397,8 +397,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.managers.taggable]
 check_untyped_defs = False
-[mypy-galaxy.managers.collections_util]
-check_untyped_defs = False
 [mypy-galaxy.jobs.splitters.multi]
 check_untyped_defs = False
 [mypy-galaxy.datatypes.display_applications.application]

From 3e755917fddb706264b1dfe61fda752193f0ff1d Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Thu, 11 Nov 2021 17:09:35 +0100
Subject: [PATCH 09/14] improve typing of lib/galaxy/managers/collections.py.

---
 lib/galaxy/managers/collections.py | 24 ++++++++++++++++++------
 setup.cfg                          |  2 --
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py
index bac6a0266fe1..c4007dc1a8be 100644
--- a/lib/galaxy/managers/collections.py
+++ b/lib/galaxy/managers/collections.py
@@ -1,4 +1,5 @@
 import logging
+from typing import Any, Dict, List, Union
 
 from sqlalchemy.orm import joinedload, Query
 
@@ -149,10 +150,16 @@ def create(self, trans, parent, name, collection_type, element_identifiers=None,
 
     def _create_instance_for_collection(self, trans, parent, name, dataset_collection, implicit_output_name=None, implicit_inputs=None, tags=None, set_hid=True, flush=True):
         if isinstance(parent, model.History):
-            dataset_collection_instance = model.HistoryDatasetCollectionAssociation(
+            dataset_collection_instance: Union[
+                model.HistoryDatasetCollectionAssociation,
+                model.LibraryDatasetCollectionAssociation,
+            ] = model.HistoryDatasetCollectionAssociation(
                 collection=dataset_collection,
                 name=name,
             )
+            assert isinstance(
+                dataset_collection_instance, model.HistoryDatasetCollectionAssociation
+            )
             if implicit_inputs:
                 for input_name, input_collection in implicit_inputs:
                     dataset_collection_instance.add_implicit_input_collection(input_name, input_collection)
@@ -549,7 +556,7 @@ def apply_rules(self, hdca, rule_set, handle_dataset):
     def _build_elements_from_rule_data(self, collection_type_description, rule_set, data, sources, handle_dataset):
         identifier_columns = rule_set.identifier_columns
         mapping_as_dict = rule_set.mapping_as_dict
-        elements = {}
+        elements: Dict[str, Any] = {}
         for data_index, row_data in enumerate(data):
             # For each row, find place in depth for this element.
             collection_type_at_depth = collection_type_description
@@ -592,18 +599,23 @@ def _build_elements_from_rule_data(self, collection_type_description, rule_set,
                         found = True
 
                     if not found:
-                        sub_collection = {}
+                        sub_collection: Dict[str, Any] = {}
                         sub_collection["src"] = "new_collection"
                         sub_collection["collection_type"] = collection_type_at_depth.collection_type
                         sub_collection["elements"] = {}
-                        elements_at_depth[identifier] = sub_collection
-                        elements_at_depth = sub_collection["elements"]
+                        elements_at_depth[
+                            identifier
+                        ] = sub_collection  # ??? but this is overridenn on the next line??
+                        elements_at_depth = sub_collection[
+                            "elements"
+                        ]  # ??? sub_collection is just lost?
 
         return elements
 
     def __init_rule_data(self, elements, collection_type_description, parent_identifiers=None):
         parent_identifiers = parent_identifiers or []
-        data, sources = [], []
+        data: List[List[str]] = []
+        sources: List[Dict[str, str]] = []
         for element in elements:
             element_object = element.element_object
             identifiers = parent_identifiers + [element.element_identifier]
diff --git a/setup.cfg b/setup.cfg
index 4e7c15ff49d1..7ea13e04dc5d 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -587,8 +587,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.managers.workflows]
 check_untyped_defs = False
-[mypy-galaxy.managers.collections]
-check_untyped_defs = False
 [mypy-galaxy.jobs.manager]
 check_untyped_defs = False
 [mypy-galaxy.tool_shed.galaxy_install.installed_repository_manager]

From e0fb5d3facd38e16e751832f4e162eb8613a485d Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Thu, 11 Nov 2021 17:28:22 +0100
Subject: [PATCH 10/14] improve typing of lib/galaxy/managers/tools.py.

---
 lib/galaxy/managers/tools.py | 5 +++++
 setup.cfg                    | 2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/galaxy/managers/tools.py b/lib/galaxy/managers/tools.py
index d99e8c6af891..8cb347f3dacc 100644
--- a/lib/galaxy/managers/tools.py
+++ b/lib/galaxy/managers/tools.py
@@ -1,4 +1,5 @@
 import logging
+from typing import TYPE_CHECKING
 from uuid import uuid4
 
 from sqlalchemy import sql
@@ -11,6 +12,9 @@
 
 log = logging.getLogger(__name__)
 
+if TYPE_CHECKING:
+    from galaxy.managers.base import OrmFilterParsersType
+
 
 class DynamicToolManager(ModelManager):
     """ Manages dynamic tools stored in Galaxy's database.
@@ -103,6 +107,7 @@ def deactivate(self, dynamic_tool):
 
 
 class ToolFilterMixin:
+    orm_filter_parsers: "OrmFilterParsersType"
 
     def create_tool_filter(self, attr, op, val):
 
diff --git a/setup.cfg b/setup.cfg
index 7ea13e04dc5d..8eb64b625473 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -549,8 +549,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.managers.users]
 check_untyped_defs = False
-[mypy-galaxy.managers.tools]
-check_untyped_defs = False
 [mypy-galaxy.managers.ratable]
 check_untyped_defs = False
 [mypy-galaxy.tools.actions.upload]

From cc3ece8ddc3ffe4c249a9f9df6b5853e4e9ecb90 Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Thu, 11 Nov 2021 17:44:27 +0100
Subject: [PATCH 11/14] No more changes needed to check_untyped_defs of
 galaxy.managers.workflows

---
 setup.cfg | 2 --
 1 file changed, 2 deletions(-)

diff --git a/setup.cfg b/setup.cfg
index 8eb64b625473..a5d24363d301 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -583,8 +583,6 @@ check_untyped_defs = False
 check_untyped_defs = False
 [mypy-galaxy.workflow.scheduling_manager]
 check_untyped_defs = False
-[mypy-galaxy.managers.workflows]
-check_untyped_defs = False
 [mypy-galaxy.jobs.manager]
 check_untyped_defs = False
 [mypy-galaxy.tool_shed.galaxy_install.installed_repository_manager]

From 0dd7bc740bd56e65719ff0bb6517c4f1d06f35ec Mon Sep 17 00:00:00 2001
From: "Michael R. Crusoe" <michael.crusoe@gmail.com>
Date: Thu, 11 Nov 2021 18:19:19 +0100
Subject: [PATCH 12/14] thank you test_galaxy_packages

---
 lib/galaxy/tools/__init__.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py
index 679fa44fdedc..f9907bae2d57 100644
--- a/lib/galaxy/tools/__init__.py
+++ b/lib/galaxy/tools/__init__.py
@@ -129,6 +129,7 @@
 
 if TYPE_CHECKING:
     from galaxy.tools.actions.metadata import SetMetadataToolAction
+    from galaxy.managers.jobs import JobSearch
 
 log = logging.getLogger(__name__)
 
@@ -538,6 +539,8 @@ class Tool(Dictifiable):
     dict_collection_visible_keys = ['id', 'name', 'version', 'description', 'labels']
     __help: Optional[threading.Lock]
     __help_by_page: Union[threading.Lock, List[str]]
+    job_search: 'JobSearch'
+    version: str
 
     def __init__(self, config_file, tool_source, app, guid=None, repository_id=None, tool_shed_repository=None, allow_code_files=True, dynamic=False):
         """Load a tool from the config named by `config_file`"""
@@ -587,7 +590,6 @@ def __init__(self, config_file, tool_source, app, guid=None, repository_id=None,
         # guid attribute since it is useful to have.
         self.guid = guid
         self.old_id = None
-        self.version = None
         self.python_template_version = None
         self._lineage = None
         self.dependencies = []
@@ -2604,7 +2606,7 @@ def exec_before_job(self, app, inp_data, out_data, param_dict=None):
             json_params['output_data'].append(data_dict)
             if json_filename is None:
                 json_filename = file_name
-        if not json_filename:
+        if json_filename is None:
             raise Exception(
                 "Must call 'exec_before_job' with 'out_data' containing at least one entry."
             )
@@ -2759,7 +2761,7 @@ def exec_before_job(self, app, inp_data, out_data, param_dict=None):
             json_params['output_data'].append(data_dict)
             if json_filename is None:
                 json_filename = file_name
-        if not json_filename:
+        if json_filename is None:
             raise Exception(
                 "Must call 'exec_before_job' with 'out_data' containing at least one entry."
             )

From 8a6edc7557d32acd9bc5b88be0a5198082019360 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Fri, 12 Nov 2021 12:21:19 +0100
Subject: [PATCH 13/14] Fix serialization of library dataset collections

---
 lib/galaxy/managers/collections_util.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/managers/collections_util.py b/lib/galaxy/managers/collections_util.py
index f61a709f4e0a..84e2a2ca39e7 100644
--- a/lib/galaxy/managers/collections_util.py
+++ b/lib/galaxy/managers/collections_util.py
@@ -111,7 +111,7 @@ def dictify_dataset_collection_instance(dataset_collection_instance, parent, sec
         encoded_history_id = security.encode_id(parent.id)
         dict_value['url'] = web.url_for('history_content_typed', history_id=encoded_history_id, id=encoded_id, type="dataset_collection")
     elif isinstance(parent, model.LibraryFolder):
-        encoded_library_id = security.encode_id(parent.library.id)  # type: ignore
+        encoded_library_id = security.encode_id(parent.library_root.id)
         encoded_folder_id = security.encode_id(parent.id)
         # TODO: Work in progress - this end-point is not right yet...
         dict_value['url'] = web.url_for('library_content', library_id=encoded_library_id, id=encoded_id, folder_id=encoded_folder_id)

From 3a2605963716726602610a0f42d08987ca015099 Mon Sep 17 00:00:00 2001
From: mvdbeek <m.vandenbeek@gmail.com>
Date: Fri, 12 Nov 2021 12:23:48 +0100
Subject: [PATCH 14/14] Minor cosmetics that hopefully clarifies what is going
 on here

---
 lib/galaxy/managers/collections.py | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py
index c4007dc1a8be..3318d3f0aa9e 100644
--- a/lib/galaxy/managers/collections.py
+++ b/lib/galaxy/managers/collections.py
@@ -599,16 +599,15 @@ def _build_elements_from_rule_data(self, collection_type_description, rule_set,
                         found = True
 
                     if not found:
+                        # Create a new collection whose elements are defined in the next loop
                         sub_collection: Dict[str, Any] = {}
                         sub_collection["src"] = "new_collection"
                         sub_collection["collection_type"] = collection_type_at_depth.collection_type
                         sub_collection["elements"] = {}
-                        elements_at_depth[
-                            identifier
-                        ] = sub_collection  # ??? but this is overridenn on the next line??
-                        elements_at_depth = sub_collection[
-                            "elements"
-                        ]  # ??? sub_collection is just lost?
+                        # Update elements with new collection
+                        elements_at_depth[identifier] = sub_collection
+                        # Subsequent loop fills elements of newly created collection
+                        elements_at_depth = sub_collection["elements"]
 
         return elements