diff --git a/lib/galaxy/tools/parameters/history_query.py b/lib/galaxy/model/dataset_collections/query.py similarity index 76% rename from lib/galaxy/tools/parameters/history_query.py rename to lib/galaxy/model/dataset_collections/query.py index 4e6ebf11c02..0fd2413bfe5 100644 --- a/lib/galaxy/tools/parameters/history_query.py +++ b/lib/galaxy/model/dataset_collections/query.py @@ -1,8 +1,30 @@ import logging +from typing import ( + List, + Optional, +) + +from typing_extensions import Protocol log = logging.getLogger(__name__) +# protocols for things that look like Galaxy model HDCAs and collections for this module +class CollectionLike(Protocol): + collection_type: str + + +class HdcaLike(Protocol): + collection: CollectionLike + + +class DataCollectionParameterLike(Protocol): + + @property + def collection_types(self) -> Optional[List[str]]: + """Return a list of collection type strings the parameter accepts.""" + + class HistoryQuery: """An object for describing the collections to pull out of a history, used by DataCollectionToolParameter. @@ -17,7 +39,7 @@ class HistoryQuery: return HistoryQuery(**kwargs) @staticmethod - def from_collection_types(collection_types, collection_type_descriptions): + def from_collection_types(collection_types: Optional[List[str]], collection_type_descriptions): if collection_types: collection_type_descriptions = [ collection_type_descriptions.for_collection_type(t) for t in collection_types @@ -33,12 +55,12 @@ class HistoryQuery: return HistoryQuery(**kwargs) @staticmethod - def from_parameter(param, collection_type_descriptions): + def from_parameter(param: DataCollectionParameterLike, collection_type_descriptions): """Take in a tool parameter element.""" collection_types = param.collection_types return HistoryQuery.from_collection_types(collection_types, collection_type_descriptions) - def direct_match(self, hdca): + def direct_match(self, hdca: HdcaLike) -> bool: collection_type_descriptions = self.collection_type_descriptions if collection_type_descriptions is not None: for collection_type_description in collection_type_descriptions: @@ -48,7 +70,7 @@ class HistoryQuery: return True - def can_map_over(self, hdca): + def can_map_over(self, hdca: HdcaLike): collection_type_descriptions = self.collection_type_descriptions if collection_type_descriptions is None: return False diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 2bf69003eba..dbe2e7e532b 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -41,7 +41,10 @@ from galaxy.model import ( HistoryDatasetCollectionAssociation, LibraryDatasetDatasetAssociation, ) -from galaxy.model.dataset_collections import builder +from galaxy.model.dataset_collections import ( + builder, + query, +) from galaxy.schema.fetch_data import FilesPayload from galaxy.tool_util.parameters.factory import get_color_value from galaxy.tool_util.parser import get_input_source as ensure_input_source @@ -70,7 +73,6 @@ from galaxy.util.hash_util import HASH_NAMES from galaxy.util.rules_dsl import RuleSet from . import ( dynamic_options, - history_query, validation, ) from .dataset_matcher import get_dataset_matcher_factory @@ -2433,7 +2435,7 @@ class DataToolParameter(BaseDataToolParameter): assert self.multiple dataset_collection_type_descriptions = trans.app.dataset_collection_manager.collection_type_descriptions # If multiple data parameter, treat like a list parameter. - return history_query.HistoryQuery.from_collection_type("list", dataset_collection_type_descriptions) + return query.HistoryQuery.from_collection_type("list", dataset_collection_type_descriptions) class DataCollectionToolParameter(BaseDataToolParameter): @@ -2459,12 +2461,12 @@ class DataCollectionToolParameter(BaseDataToolParameter): ) @property - def collection_types(self): + def collection_types(self) -> Optional[List[str]]: return self._collection_types def _history_query(self, trans): dataset_collection_type_descriptions = trans.app.dataset_collection_manager.collection_type_descriptions - return history_query.HistoryQuery.from_parameter(self, dataset_collection_type_descriptions) + return query.HistoryQuery.from_parameter(self, dataset_collection_type_descriptions) def match_collections(self, trans, history, dataset_collection_matcher): dataset_collections = trans.app.dataset_collection_manager.history_dataset_collections( diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index c894acc34d1..f7b0c188c64 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -42,6 +42,7 @@ from galaxy.model import ( ) from galaxy.model.base import ensure_object_added_to_session from galaxy.model.dataset_collections import matching +from galaxy.model.dataset_collections.query import HistoryQuery from galaxy.schema.invocation import ( CancelReason, FailureReason, @@ -94,7 +95,6 @@ from galaxy.tools.parameters.grouping import ( ConditionalWhen, Repeat, ) -from galaxy.tools.parameters.history_query import HistoryQuery from galaxy.tools.parameters.options import ParameterOption from galaxy.tools.parameters.populate_model import populate_model from galaxy.tools.parameters.workflow_utils import ( @@ -603,6 +603,9 @@ class WorkflowModule: effective_input_collection_type, dataset_collection_type_descriptions, ) + if not progress.subworkflow_structure and history_query.direct_match(data): + continue + subcollection_type_description = history_query.can_map_over(data) or None if subcollection_type_description: subcollection_type_list = subcollection_type_description.collection_type.split(":") diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 6920bb75c19..65ec8edc99c 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -4960,6 +4960,57 @@ test_data: hda_content = self.dataset_populator.get_history_dataset_content(history_id, content_id=hda_id) assert hda_content.strip() == "1" + @skip_without_tool("identifier_multiple") + def test_invocation_double_map_over(self, history_id): + summary = self._run_workflow( + """ +class: GalaxyWorkflow +inputs: + input_collection: + collection_type: list:list + type: collection +outputs: + main_out: + outputSource: subworkflow/sub_out +steps: + subworkflow: + in: + data_input: input_collection + run: + class: GalaxyWorkflow + inputs: + data_input: + type: data + outputs: + sub_out: + outputSource: output_step/output1 + steps: + intermediate_step: + tool_id: identifier_single + in: + input1: data_input + output_step: + tool_id: identifier_single + in: + input1: intermediate_step/output1 +test_data: + input_collection: + collection_type: list:list + """, + history_id=history_id, + assert_ok=True, + wait=True, + ) + invocation = self.workflow_populator.get_invocation(summary.invocation_id) + # For consistency and conditional subworkflow steps this really needs to remain + # a collection and not get reduced. + assert "main_out" in invocation["output_collections"], invocation + hdca_details = self.dataset_populator.get_history_collection_details(history_id) + assert hdca_details["collection_type"] == "list:list" + elements = hdca_details["elements"] + assert len(elements) == 1 + assert elements[0]["element_identifier"] == "test_level_1" + @skip_without_tool("identifier_multiple") def test_invocation_map_over_inner_collection(self, history_id): summary = self._run_workflow( @@ -5074,6 +5125,53 @@ test_data: assert elements[0]["element_identifier"] == "test_level_1" assert elements[0]["element_type"] == "hda" + @skip_without_tool("identifier_multiple") + def test_invocation_double_map_over_inner_collection_with_tool_collection_input(self, history_id): + summary = self._run_workflow( + """ +class: GalaxyWorkflow +inputs: + input_collection: + collection_type: list:list:list + type: collection +outputs: + main_out: + outputSource: subworkflow/sub_out +steps: + subworkflow: + in: + list_input: input_collection + run: + class: GalaxyWorkflow + inputs: + list_input: + type: collection + collection_type: list + outputs: + sub_out: + outputSource: output_step/output1 + steps: + output_step: + tool_id: identifier_all_collection_types + in: + input1: list_input +test_data: + input_collection: + collection_type: list:list:list + """, + history_id=history_id, + assert_ok=True, + wait=True, + ) + invocation = self.workflow_populator.get_invocation(summary.invocation_id) + assert "main_out" in invocation["output_collections"], invocation + input_hdca_details = self.dataset_populator.get_history_collection_details( + history_id, content_id=invocation["inputs"]["0"]["id"] + ) + assert input_hdca_details["collection_type"] == "list:list:list" + assert len(input_hdca_details["elements"]) == 1 + assert input_hdca_details["elements"][0]["element_identifier"] == "test_level_2" + @skip_without_tool("cat") def test_pause_outputs_with_deleted_inputs(self): self._deleted_inputs_workflow(purge=False) diff --git a/lib/galaxy_test/workflow/subcollection_rank_sorting.gxwf-tests.yml b/lib/galaxy_test/workflow/subcollection_rank_sorting.gxwf-tests.yml new file mode 100644 index 00000000000..c9a4c74e4e8 --- /dev/null +++ b/lib/galaxy_test/workflow/subcollection_rank_sorting.gxwf-tests.yml @@ -0,0 +1,51 @@ +- doc: | + Test to verify a tool that takes in a list or nested list is sent the most + specific list possible during workflow execution (the nested list). This should + not trigger subcollection mapping. + job: + input: + type: collection + collection_type: list:list + outputs: + out: + asserts: + - that: has_text + text: "Nested List" +- doc: | + Once again with mapping, should reduce the result into a flat list. + job: + input: + type: collection + collection_type: list:list:list + outputs: + out: + class: Collection + collection_type: list + elements: + test_level_2: + asserts: + - that: has_line + line: "Nested List" + - that: has_line + line: "identifier is test_level_1" + +- doc: | + A test case with two levels of mapping. The inner two levels should be consumed and the outer + two levels should be mapped over. + job: + input: + type: collection + collection_type: list:list:list:list + outputs: + out: + class: Collection + collection_type: list:list + elements: + test_level_3: + elements: + test_level_2: + asserts: + - that: has_text + text: "Nested List" + - that: has_line + line: "identifier is test_level_1" diff --git a/lib/galaxy_test/workflow/subcollection_rank_sorting.gxwf.yml b/lib/galaxy_test/workflow/subcollection_rank_sorting.gxwf.yml new file mode 100644 index 00000000000..a0616bbc0ac --- /dev/null +++ b/lib/galaxy_test/workflow/subcollection_rank_sorting.gxwf.yml @@ -0,0 +1,13 @@ +class: GalaxyWorkflow +inputs: + input: + type: collection + collection_type: list:list +outputs: + out: + outputSource: list_handling/out1 +steps: + list_handling: + tool_id: collection_list_or_nested_list_input + in: + f1: input diff --git a/lib/galaxy_test/workflow/subcollection_rank_sorting_paired.gxwf-tests.yml b/lib/galaxy_test/workflow/subcollection_rank_sorting_paired.gxwf-tests.yml new file mode 100644 index 00000000000..d12a7fb80e2 --- /dev/null +++ b/lib/galaxy_test/workflow/subcollection_rank_sorting_paired.gxwf-tests.yml @@ -0,0 +1,51 @@ +- doc: | + Test to verify a tool that takes in a paired or list of paired is sent the most + specific list possible during workflow execution (the list of paired datasets). This should + not trigger subcollection mapping. + job: + input: + type: collection + collection_type: list:paired + outputs: + out: + asserts: + - that: has_text + text: "collection_type" + +- doc: | + Once again with mapping, should reduce the result into a flat list. + job: + input: + type: collection + collection_type: list:list:paired + outputs: + out: + class: Collection + collection_type: list + elements: + test_level_2: + asserts: + - that: has_text + text: "identifier is test_level_1" + - that: has_line + line: "collection_type" + +- doc: | + Once again with double mapping, should reduce the result into a nested list (list:list). + job: + input: + type: collection + collection_type: list:list:list:paired + outputs: + out: + class: Collection + collection_type: list:list + elements: + test_level_3: + elements: + test_level_2: + asserts: + - that: has_text + text: "identifier is test_level_1" + - that: has_line + line: "collection_type" diff --git a/lib/galaxy_test/workflow/subcollection_rank_sorting_paired.gxwf.yml b/lib/galaxy_test/workflow/subcollection_rank_sorting_paired.gxwf.yml new file mode 100644 index 00000000000..422f180a163 --- /dev/null +++ b/lib/galaxy_test/workflow/subcollection_rank_sorting_paired.gxwf.yml @@ -0,0 +1,13 @@ +class: GalaxyWorkflow +inputs: + input: + type: collection + collection_type: list:paired +outputs: + out: + outputSource: list_handling/out1 +steps: + list_handling: + tool_id: collection_paired_or_list_paired_input + in: + f1: input diff --git a/lib/galaxy_test/workflow/triply_nested_list_mapping.gxwf-tests.yml b/lib/galaxy_test/workflow/triply_nested_list_mapping.gxwf-tests.yml new file mode 100644 index 00000000000..0c2f03f7284 --- /dev/null +++ b/lib/galaxy_test/workflow/triply_nested_list_mapping.gxwf-tests.yml @@ -0,0 +1,32 @@ +- doc: | + Test that a basic list:list:list parameter is consumed directly by the tool + and not mapped over. + job: + input: + type: collection + collection_type: list:list:list + outputs: + out: + asserts: + - that: has_text + text: "identifier is test_level_2" + - that: has_text + text: "collection_type" + +- doc: | + Test that mapping over a list:list:list input parameter. + job: + input: + type: collection + collection_type: list:list:list:list + outputs: + out: + class: Collection + collection_type: list + elements: + test_level_3: + asserts: + - that: has_text + text: "identifier is test_level_2" + - that: has_text + text: "collection_type" diff --git a/lib/galaxy_test/workflow/triply_nested_list_mapping.gxwf.yml b/lib/galaxy_test/workflow/triply_nested_list_mapping.gxwf.yml new file mode 100644 index 00000000000..1e20596a24f --- /dev/null +++ b/lib/galaxy_test/workflow/triply_nested_list_mapping.gxwf.yml @@ -0,0 +1,13 @@ +class: GalaxyWorkflow +inputs: + input: + type: collection + collection_type: list:list:list +outputs: + out: + outputSource: list_handling/out1 +steps: + list_handling: + tool_id: collection_list_2x_or_3x_nested + in: + f1: input diff --git a/run_tests.sh b/run_tests.sh index 11c14635390..ea6c3f864b4 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -409,7 +409,7 @@ do framework_test=1 shift 1 ;; - -w|--framework-workflows) + -w|-framework-workflows|--framework-workflows) marker="workflow" report_file="run_framework_workflows_tests.html" framework_workflows_test=1 diff --git a/test/functional/tools/collection_list_2x_or_3x_nested.xml b/test/functional/tools/collection_list_2x_or_3x_nested.xml new file mode 100644 index 00000000000..9307b898d25 --- /dev/null +++ b/test/functional/tools/collection_list_2x_or_3x_nested.xml @@ -0,0 +1,17 @@ + + > "$out1"; + #set $element = $f1[$key] + echo 'collection_type<${element.collection.collection_type}>' >> "$out1"; + #end for# + ]]> + + + + + + + + + diff --git a/test/functional/tools/collection_list_or_nested_list_input.xml b/test/functional/tools/collection_list_or_nested_list_input.xml new file mode 100644 index 00000000000..f1503749e7c --- /dev/null +++ b/test/functional/tools/collection_list_or_nested_list_input.xml @@ -0,0 +1,22 @@ + + > "$out1"; + #set $element = $f1[$key] + + #if $element.is_collection: + echo "Nested List" >> "$out1"; + #else + echo "Simple Dataset" >> "$out1"; + #end if + #end for# + ]]> + + + + + + + + + diff --git a/test/functional/tools/collection_paired_or_list_paired_input.xml b/test/functional/tools/collection_paired_or_list_paired_input.xml new file mode 100644 index 00000000000..2acfb829e7f --- /dev/null +++ b/test/functional/tools/collection_paired_or_list_paired_input.xml @@ -0,0 +1,21 @@ + + > "$out1"; + #set $element = $f1[$key] + #if $element.is_collection: + echo 'collection_type<${element.collection.collection_type}>' >> "$out1"; + #else: + echo 'Simple dataset' >> "$out1"; + #end if# + #end for# + ]]> + + + + + + + + + diff --git a/test/functional/tools/sample_tool_conf.xml b/test/functional/tools/sample_tool_conf.xml index 9e4fe18b328..064dda350f5 100644 --- a/test/functional/tools/sample_tool_conf.xml +++ b/test/functional/tools/sample_tool_conf.xml @@ -210,6 +210,9 @@ + + + diff --git a/test/unit/data/dataset_collections/test_matching.py b/test/unit/data/dataset_collections/test_matching.py index a7bb1985569..176332821ce 100644 --- a/test/unit/data/dataset_collections/test_matching.py +++ b/test/unit/data/dataset_collections/test_matching.py @@ -1,5 +1,6 @@ from galaxy.model.dataset_collections import ( matching, + query, registry, type_description, ) @@ -17,14 +18,7 @@ def test_lists_of_same_cardinality_match(): def test_nested_lists_match(): - nested_list = list_instance( - elements=[ - pair_element("data1"), - pair_element("data2"), - pair_element("data3"), - ], - collection_type="list:paired", - ) + nested_list = nested_list = example_list_of_paired_datasets() assert_can_match(nested_list, nested_list) @@ -42,19 +36,60 @@ def test_lists_of_different_cardinality_do_not_match(): def test_valid_collection_subcollection_matching(): flat_list = list_instance(ids=["data1", "data2", "data3"]) - nested_list = list_instance( - elements=[ - pair_element("data11"), - pair_element("data21"), - pair_element("data31"), - ], - collection_type="list:paired", - ) + nested_list = example_list_of_paired_datasets() assert_cannot_match(flat_list, nested_list) assert_cannot_match(nested_list, flat_list) assert_can_match((nested_list, "paired"), flat_list) +def test_query_can_match_list_to_list(): + flat_list = list_instance(ids=["data1", "data2", "data3"]) + q = query.HistoryQuery.from_collection_types(["list"], TYPE_DESCRIPTION_FACTORY) + assert q.can_map_over(flat_list) is False + assert q.direct_match(flat_list) is True + + +def test_query_can_match_list_of_paireds_to_paired(): + list_of_paired_datasets = example_list_of_paired_datasets() + q = query.HistoryQuery.from_collection_types(["paired"], TYPE_DESCRIPTION_FACTORY) + assert q.can_map_over(list_of_paired_datasets).collection_type == "paired" + + +def test_query_can_match_list_of_lists_to_paired(): + list_of_lists = example_list_of_lists() + q = query.HistoryQuery.from_collection_types(["paired"], TYPE_DESCRIPTION_FACTORY) + assert not q.can_map_over(list_of_lists) + assert not q.direct_match(list_of_lists) + + +def test_query_can_match_list_of_lists_to_list(): + list_of_lists = example_list_of_lists() + q = query.HistoryQuery.from_collection_types(["list"], TYPE_DESCRIPTION_FACTORY) + assert q.can_map_over(list_of_lists).collection_type == "list" + assert not q.direct_match(list_of_lists) + + +def test_query_can_match_list_of_paireds_to_list_or_paired(): + list_of_paired_datasets = example_list_of_paired_datasets() + q = query.HistoryQuery.from_collection_types(["list", "paired"], TYPE_DESCRIPTION_FACTORY) + assert q.can_map_over(list_of_paired_datasets).collection_type == "paired" + assert q.direct_match(list_of_paired_datasets) is False + + +def test_query_can_match_list_of_lists_to_list_or_paired(): + list_of_lists = example_list_of_lists() + q = query.HistoryQuery.from_collection_types(["list", "paired"], TYPE_DESCRIPTION_FACTORY) + assert q.can_map_over(list_of_lists).collection_type == "list" + assert q.direct_match(list_of_lists) is False + + +def test_query_always_direct_match_if_no_collection_type_on_input_specified(): + list_of_lists = example_list_of_lists() + q = query.HistoryQuery.from_collection_types([], TYPE_DESCRIPTION_FACTORY) + assert q.can_map_over(list_of_lists) is False + assert q.direct_match(list_of_lists) is True + + def assert_can_match(*items): to_match = build_collections_to_match(*items) matching.MatchingCollections.for_collections(to_match, TYPE_DESCRIPTION_FACTORY) @@ -82,6 +117,27 @@ def build_collections_to_match(*items): return to_match +def example_list_of_paired_datasets(): + return list_instance( + elements=[ + pair_element("data1"), + pair_element("data2"), + pair_element("data3"), + ], + collection_type="list:paired", + ) + + +def example_list_of_lists(): + return list_instance( + elements=[ + list_instance(), + list_instance(), + ], + collection_type="list:list", + ) + + def pair_element(element_identifier): return collection_element(element_identifier, pair_instance().collection)