From f99299f8332a75a6075f49176770078be9cda571 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 2 Feb 2018 14:46:17 -0500 Subject: [PATCH 1/2] Pre-compute more collection state during mapping. Still dealing with fallout from 78babab6455f4cd1c51aed0b1ddf6b4b29ad6d03. After that commit we were treating structured_like collections like collections with unknown structure and not dealing with collection_type_source information at all when building backbones for new collections during mapping. --- lib/galaxy/dataset_collections/structure.py | 37 ++++++++++++++------- lib/galaxy/tools/execute.py | 19 ++++++----- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/lib/galaxy/dataset_collections/structure.py b/lib/galaxy/dataset_collections/structure.py index 0caf7f2fac5..fff4f082a20 100644 --- a/lib/galaxy/dataset_collections/structure.py +++ b/lib/galaxy/dataset_collections/structure.py @@ -23,11 +23,8 @@ class Leaf(object): def clone(self): return self - def multiply(self, other_structure, uninitialized=False): - if not uninitialized: - return other_structure.clone() - else: - return UnitializedTree(other_structure.collection_type_description) + def multiply(self, other_structure): + return other_structure.clone() def sliced_collection_type(self, collection): return input @@ -59,7 +56,7 @@ class UnitializedTree(BaseTree): def __len__(self): raise Exception("Unknown length") - def multiply(self, other_structure, uninitialized=False): + def multiply(self, other_structure): if other_structure.is_leaf: return self.clone() @@ -154,14 +151,14 @@ class Tree(BaseTree): element_identifiers=element_identifiers, ) - def multiply(self, other_structure, uninitialized=False): + def multiply(self, other_structure): if other_structure.is_leaf: return self.clone() new_collection_type = self.collection_type_description.multiply(other_structure.collection_type_description) new_children = [] for (identifier, structure) in self.children: - new_children.append((identifier, structure.multiply(other_structure, uninitialized=uninitialized))) + new_children.append((identifier, structure.multiply(other_structure))) return Tree(new_children, new_collection_type) @@ -173,19 +170,35 @@ class Tree(BaseTree): return "Tree[collection_type=%s,children=%s]" % (self.collection_type_description, ",".join(map(lambda identifier_and_element: "%s=%s" % (identifier_and_element[0], identifier_and_element[1]), self.children))) -def tool_output_to_structure(get_sliced_input_collection_type, tool_output, collections_manager): +def tool_output_to_structure(get_sliced_input_collection_structure, tool_output, collections_manager): if not tool_output.collection: tree = leaf else: + collection_type_descriptions = collections_manager.collection_type_descriptions # Okay this is ToolCollectionOutputStructure not a Structure - different # concepts of structure. structured_like = tool_output.structure.structured_like + collection_type = tool_output.structure.collection_type if structured_like: - collection_type = get_sliced_input_collection_type(structured_like) + tree = get_sliced_input_collection_structure(structured_like) + if collection_type and tree.collection_type_description.collection_type != collection_type: + # See tool paired_collection_map_over_structured_like - type should + # override structured_like if they disagree. + tree = UnitializedTree(collection_type_descriptions.for_collection_type(collection_type)) else: - collection_type = tool_output.structure.collection_type - tree = UnitializedTree(collection_type) + # Can't pre-compute the structure in this case, see if we can find a collection type. + if collection_type is None and tool_output.structure.collection_type_source: + collection_type = get_sliced_input_collection_structure(tool_output.structure.collection_type_source).collection_type_description.collection_type + if not collection_type: + raise Exception("Failed to determine collection type for mapping over output %s" % tool_output.name) + + tree = UnitializedTree(collection_type_descriptions.for_collection_type(collection_type)) + + if not tree.children_known and tree.collection_type_description.collection_type == "paired": + # TODO: We don't need to return unitializedtree for pairs I think, we should build + # a paired tree for the known structure here. + pass return tree diff --git a/lib/galaxy/tools/execute.py b/lib/galaxy/tools/execute.py index 880bc4a38c3..0a80ce256ca 100644 --- a/lib/galaxy/tools/execute.py +++ b/lib/galaxy/tools/execute.py @@ -12,7 +12,7 @@ import six from six.moves.queue import Queue from galaxy import model -from galaxy.dataset_collections.structure import tool_output_to_structure +from galaxy.dataset_collections.structure import get_structure, tool_output_to_structure from galaxy.tools.actions import filter_output, on_text_for_names, ToolExecutionCache from galaxy.tools.parser import ToolOutputCollectionPart from galaxy.util import ExecutionTimer @@ -210,13 +210,14 @@ class ExecutionTracker(object): return output_collection_name - def sliced_input_collection_type(self, input_name): + def sliced_input_collection_structure(self, input_name): + input_collection = self.example_params[input_name] + collection_type_description = self.trans.app.dataset_collections_service.collection_type_descriptions.for_collection_type(input_collection.collection.collection_type) + subcollection_mapping_type = None if self.is_implicit_input(input_name): subcollection_mapping_type = self.collection_info.subcollection_mapping_type(input_name) - return subcollection_mapping_type - # return self.collection_info.structure.sliced_input_collection_type(self.implicit_inputs[input_name]) - else: - return self.mapping_params.param_template[input_name].collection.collection_type + + return get_structure(input_collection, collection_type_description, leaf_subcollection_type=subcollection_mapping_type) def _structure_for_output(self, trans, tool_output): structure = self.collection_info.structure @@ -240,12 +241,14 @@ class ExecutionTracker(object): def _mapped_output_structure(self, trans, tool_output): collections_manager = trans.app.dataset_collections_service - output_structure = tool_output_to_structure(self.sliced_input_collection_type, tool_output, collections_manager) + output_structure = tool_output_to_structure(self.sliced_input_collection_structure, tool_output, collections_manager) + # self.collection_info.structure - the mapping structure with default_identifier_source + # used to determine the identifiers to use. mapping_structure = self._structure_for_output(trans, tool_output) # Output structure may not be known, but input structure must be, # otherwise this step of the workflow shouldn't have been scheduled # or the tool should not have been executable on this input. - mapped_output_structure = mapping_structure.multiply(output_structure, uninitialized=True) + mapped_output_structure = mapping_structure.multiply(output_structure) return mapped_output_structure def ensure_implicit_collections_populated(self, history, params): From 51e030f4f3b608710adbba9ed4647db3d20196cb Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 2 Feb 2018 15:31:28 -0500 Subject: [PATCH 2/2] Bring in @mvdbeek's test cases from #5431. --- test/api/test_tools.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/api/test_tools.py b/test/api/test_tools.py index 8da38b4388a..bca7e77c18e 100644 --- a/test/api/test_tools.py +++ b/test/api/test_tools.py @@ -1263,6 +1263,44 @@ class ToolsTestCase(api.ApiTestCase): # on server. assert run_response.status_code >= 400 + @skip_without_tool("__FILTER_FROM_FILE__") + def test_map_over_collection_structured_like(self): + with self.dataset_populator.test_history() as history_id: + hdca_id = self.dataset_collection_populator.create_list_in_history(history_id, contents=[("A", "A"), ("B", "B")]).json()['id'] + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + inputs = { + "input": {'values': [dict(src="hdca", id=hdca_id)]}, + "how|filter_source": {'batch': True, 'values': [dict(src="hdca", id=hdca_id)]} + } + self._run("__FILTER_FROM_FILE__", history_id, inputs, assert_ok=True) + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + history_contents = self.dataset_populator._get_contents_request(history_id).json() + # We should have a final collection count of 3 (2 nested collections, plus the input collection) + new_collections = len([c for c in history_contents if c['history_content_type'] == 'dataset_collection']) - 1 + assert new_collections == 2, "Expected to generate 4 new, filtered collections, but got %d collections" % new_collections + filtered_collection = history_contents[7] + assert filtered_collection['collection_type'] == 'list:list', filtered_collection + collection_details = self.dataset_populator.get_history_collection_details(history_id, hid=filtered_collection['hid']) + assert collection_details['element_count'] == 2 + first_collection_level = collection_details['elements'][0] + assert first_collection_level['element_type'] == 'dataset_collection' + second_collection_level = first_collection_level['object'] + assert second_collection_level['collection_type'] == 'list' + assert second_collection_level['elements'][0]['element_type'] == 'hda' + + @skip_without_tool("collection_type_source") + def test_map_over_collection_type_source(self): + with self.dataset_populator.test_history() as history_id: + hdca_id = self.dataset_collection_populator.create_list_in_history(history_id, contents=[("A", "A"), ("B", "B")]).json()['id'] + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + inputs = { + "input_collect": {'values': [dict(src="hdca", id=hdca_id)]}, + "header": {'batch': True, 'values': [dict(src="hdca", id=hdca_id)]} + } + self._run("collection_type_source", history_id, inputs, assert_ok=True, wait_for_job=True) + collection_details = self.dataset_populator.get_history_collection_details(history_id, hid=4) + assert collection_details['elements'][0]['object']['elements'][0]['element_type'] == 'hda' + @skip_without_tool("multi_data_param") def test_reduce_collections_legacy(self): history_id = self.dataset_populator.new_history()