From 7490fd452046b4a4578f6401501439773cbf2f28 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 26 Jan 2018 13:19:51 -0500 Subject: [PATCH 1/3] More robust nametag discovery and propagation. - Nametags weren't being picked up on collection inputs - so for instance tools that take in a single collection and produce a dataset or file weren't propagating nametags (unless the collection contents had tags). To see this create two unnamed datasets without tags, create a pair from them, add a tag to the pair, and run the pair through the "paired_test" test tool. - Nametags weren't being propagated to the outputs of collection operations. --- lib/galaxy/model/__init__.py | 10 ++++-- lib/galaxy/tools/__init__.py | 20 ++++++----- lib/galaxy/tools/actions/__init__.py | 35 +++++++++++++------- lib/galaxy/tools/actions/model_operations.py | 11 +++--- 4 files changed, 48 insertions(+), 28 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 0b9e9c640f5..beae33c20d1 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -2455,7 +2455,7 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot self.version = self.version + 1 if self.version else 1 session.add(past_hda) - def copy(self, parent_id=None): + def copy(self, parent_id=None, copy_tags=None): """ Create a copy of this HDA. """ @@ -2474,7 +2474,7 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot copied_from_history_dataset_association=self) # update init non-keywords as well hda.purged = self.purged - + hda.copy_tags_to(copy_tags) object_session(self).add(hda) object_session(self).flush() hda.set_size() @@ -2486,6 +2486,12 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot object_session(self).flush() return hda + def copy_tags_to(self, copy_tags=None): + if copy_tags is not None: + for _, tag in copy_tags.items(): + copied_tag = tag.copy(cls=HistoryDatasetAssociationTagAssociation) + self.tags.append(copied_tag) + def copy_attributes(self, new_dataset): new_dataset.hid = self.hid diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index fca06568f46..c3e7547507a 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -2333,7 +2333,7 @@ class DatabaseOperationTool(Tool): class UnzipCollectionTool(DatabaseOperationTool): tool_type = 'unzip_collection' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, tags=None): has_collection = incoming["input"] if hasattr(has_collection, "element_type"): # It is a DCE @@ -2344,10 +2344,12 @@ class UnzipCollectionTool(DatabaseOperationTool): assert collection.collection_type == "paired" forward_o, reverse_o = collection.dataset_instances - forward, reverse = forward_o.copy(), reverse_o.copy() + forward, reverse = forward_o.copy(copy_tags=tags), reverse_o.copy(copy_tags=tags) + # TODO: rename... history.add_dataset(forward, set_hid=True) history.add_dataset(reverse, set_hid=True) + out_data["forward"] = forward out_data["reverse"] = reverse @@ -2355,7 +2357,7 @@ class UnzipCollectionTool(DatabaseOperationTool): class ZipCollectionTool(DatabaseOperationTool): tool_type = 'zip_collection' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): forward_o = incoming["input_forward"] reverse_o = incoming["input_reverse"] @@ -2374,7 +2376,7 @@ class ZipCollectionTool(DatabaseOperationTool): class MergeCollectionTool(DatabaseOperationTool): tool_type = 'merge_collection' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): input_lists = [] for incoming_repeat in incoming["inputs"]: @@ -2457,7 +2459,7 @@ class FilterFailedDatasetsTool(DatabaseOperationTool): tool_type = 'filter_failed_datasets_collection' require_dataset_ok = False - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): hdca = incoming["input"] assert hdca.collection.collection_type == "list" or hdca.collection.collection_type == 'list:paired' @@ -2494,7 +2496,7 @@ class FilterFailedDatasetsTool(DatabaseOperationTool): class FlattenTool(DatabaseOperationTool): tool_type = 'flatten_collection' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): hdca = incoming["input"] join_identifier = incoming["join_identifier"] new_elements = odict() @@ -2519,7 +2521,7 @@ class FlattenTool(DatabaseOperationTool): class SortTool(DatabaseOperationTool): tool_type = 'sort_collection' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): hdca = incoming["input"] sorttype = incoming["sort_type"]["sort_type"] new_elements = odict() @@ -2557,7 +2559,7 @@ class SortTool(DatabaseOperationTool): class RelabelFromFileTool(DatabaseOperationTool): tool_type = 'relabel_from_file' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): hdca = incoming["input"] how_type = incoming["how"]["how_select"] new_labels_dataset_assoc = incoming["how"]["labels"] @@ -2606,7 +2608,7 @@ class RelabelFromFileTool(DatabaseOperationTool): class FilterFromFileTool(DatabaseOperationTool): tool_type = 'filter_from_file' - def produce_outputs(self, trans, out_data, output_collections, incoming, history): + def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): hdca = incoming["input"] how_filter = incoming["how"]["how_filter"] filter_dataset_assoc = incoming["how"]["filter_source"] diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 372cadc76ae..9fb39c9457b 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -193,7 +193,22 @@ class DefaultToolAction(object): # Collect any input datasets from the incoming parameters inp_data = self._collect_input_datasets(tool, incoming, trans, history=history, current_user_roles=current_user_roles) - return history, inp_data, inp_dataset_collections + # grap tags from incoming HDAs + preserved_tags = {} + for data in inp_data.values(): + for tag in [t for t in data.tags if t.user_tname == 'name']: + preserved_tags[tag.value] = tag + + # grap tags from incoming HDCAs + for key, collection_pairs in inp_dataset_collections.items(): + for collection, _ in collection_pairs: + # if sub-collection mapping, this will be an DC not an HDCA + # (e.g. part of collection not a collection instance) and thus won't have tags. + if hasattr(collection, "tags"): + for tag in [t for t in collection.tags if t.user_tname == 'name']: + preserved_tags[tag.value] = tag + + return history, inp_data, inp_dataset_collections, preserved_tags def execute(self, tool, trans, incoming={}, return_job=False, set_output_hid=True, history=None, job_params=None, rerun_remap_job_id=None, execution_cache=None, dataset_collection_elements=None, completed_job=None): """ @@ -206,7 +221,7 @@ class DefaultToolAction(object): if execution_cache is None: execution_cache = ToolExecutionCache(trans) current_user_roles = execution_cache.current_user_roles - history, inp_data, inp_dataset_collections = self._collect_inputs(tool, trans, incoming, history, current_user_roles) + history, inp_data, inp_dataset_collections, preserved_tags = self._collect_inputs(tool, trans, incoming, history, current_user_roles) # Build name for output datasets based on tool name and input names on_text = self._get_on_text(inp_data) @@ -216,7 +231,6 @@ class DefaultToolAction(object): # format. input_ext = 'data' if tool.profile < 16.04 else "input" input_dbkey = incoming.get("dbkey", "?") - preserved_tags = {} for name, data in reversed(inp_data.items()): if not data: data = NoneDataset(datatypes_registry=app.datatypes_registry) @@ -237,9 +251,6 @@ class DefaultToolAction(object): if identifier is not None: incoming["%s|__identifier__" % name] = identifier - for tag in [t for t in data.tags if t.user_tname == 'name']: - preserved_tags[tag.value] = tag - # Collect chromInfo dataset and add as parameters to incoming (chrom_info, db_dataset) = app.genome_builds.get_chrom_info(input_dbkey, trans=trans, custom_build_hack_get_len_from_fasta_conversion=tool.id != 'CONVERTER_fasta_to_len') if db_dataset: @@ -273,6 +284,7 @@ class DefaultToolAction(object): incoming=incoming, params=wrapped_params.params, job_params=job_params, + tags=preserved_tags, ) # Keep track of parent / child relationships, we'll create all the @@ -325,8 +337,7 @@ class DefaultToolAction(object): trans.sa_session.add(data) if not completed_job: trans.app.security_agent.set_all_dataset_permissions(data.dataset, output_permissions, new=True) - for _, tag in preserved_tags.items(): - data.tags.append(tag.copy()) + data.copy_tags_to(preserved_tags) # Must flush before setting object store id currently. # TODO: optimize this. @@ -430,7 +441,6 @@ class DefaultToolAction(object): output_collections.create_collection( output=output, name=name, - tags=preserved_tags, **element_kwds ) log.info("Handled collection output named %s for tool %s %s" % (name, tool.id, handle_output_timer)) @@ -736,7 +746,7 @@ class OutputCollections(object): parameter). """ - def __init__(self, trans, history, tool, tool_action, input_collections, dataset_collection_elements, on_text, incoming, params, job_params): + def __init__(self, trans, history, tool, tool_action, input_collections, dataset_collection_elements, on_text, incoming, params, job_params, tags): self.trans = trans self.history = history self.tool = tool @@ -749,8 +759,9 @@ class OutputCollections(object): self.job_params = job_params self.out_collections = {} self.out_collection_instances = {} + self.tags = tags - def create_collection(self, output, name, tags=None, **element_kwds): + def create_collection(self, output, name, **element_kwds): input_collections = self.input_collections collections_manager = self.trans.app.dataset_collections_service collection_type = output.structure.collection_type @@ -804,7 +815,7 @@ class OutputCollections(object): name=hdca_name, collection_type=collection_type, trusted_identifiers=True, - tags=tags, + tags=self.tags, **element_kwds ) # name here is name of the output element - not name diff --git a/lib/galaxy/tools/actions/model_operations.py b/lib/galaxy/tools/actions/model_operations.py index 842a3c085d5..5633104ffe0 100644 --- a/lib/galaxy/tools/actions/model_operations.py +++ b/lib/galaxy/tools/actions/model_operations.py @@ -17,7 +17,7 @@ class ModelOperationToolAction(DefaultToolAction): execution_cache = ToolExecutionCache(trans) current_user_roles = execution_cache.current_user_roles - history, inp_data, inp_dataset_collections = self._collect_inputs(tool, trans, incoming, history, current_user_roles) + history, inp_data, inp_dataset_collections, _ = self._collect_inputs(tool, trans, incoming, history, current_user_roles) tool.check_inputs_ready(inp_data, inp_dataset_collections) @@ -26,7 +26,7 @@ class ModelOperationToolAction(DefaultToolAction): execution_cache = ToolExecutionCache(trans) current_user_roles = execution_cache.current_user_roles - history, inp_data, inp_dataset_collections = self._collect_inputs(tool, trans, incoming, history, current_user_roles) + history, inp_data, inp_dataset_collections, preserved_tags = self._collect_inputs(tool, trans, incoming, history, current_user_roles) # Build name for output datasets based on tool name and input names on_text = self._get_on_text(inp_data) @@ -47,13 +47,14 @@ class ModelOperationToolAction(DefaultToolAction): incoming=incoming, params=wrapped_params.params, job_params=job_params, + tags=preserved_tags, ) # # Create job. # job, galaxy_session = self._new_job_for_session(trans, tool, history) - self._produce_outputs(trans, tool, out_data, output_collections, incoming=incoming, history=history) + self._produce_outputs(trans, tool, out_data, output_collections, incoming=incoming, history=history, tags=preserved_tags) self._record_inputs(trans, tool, job, incoming, inp_data, inp_dataset_collections, current_user_roles) self._record_outputs(job, out_data, output_collections) job.state = job.states.OK @@ -66,7 +67,7 @@ class ModelOperationToolAction(DefaultToolAction): log.info("Calling produce_outputs, tool is %s" % tool) return job, out_data - def _produce_outputs(self, trans, tool, out_data, output_collections, incoming, history, **kwargs): - tool.produce_outputs(trans, out_data, output_collections, incoming, history=history) + def _produce_outputs(self, trans, tool, out_data, output_collections, incoming, history, tags): + tool.produce_outputs(trans, out_data, output_collections, incoming, history=history, tags=tags) trans.sa_session.add_all(out_data.values()) trans.sa_session.flush() From 6f40f7098d507895c6c6608aff6a698289f2eeb6 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 31 Jan 2018 08:20:20 -0500 Subject: [PATCH 2/3] Cleanup collection operation tagging implementation thanks to @mvdbeek comments. --- lib/galaxy/model/__init__.py | 2 +- lib/galaxy/tools/actions/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index beae33c20d1..27bd5abb3c7 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -2488,7 +2488,7 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot def copy_tags_to(self, copy_tags=None): if copy_tags is not None: - for _, tag in copy_tags.items(): + for tag in copy_tags.values(): copied_tag = tag.copy(cls=HistoryDatasetAssociationTagAssociation) self.tags.append(copied_tag) diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 9fb39c9457b..98adf24defa 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -200,7 +200,7 @@ class DefaultToolAction(object): preserved_tags[tag.value] = tag # grap tags from incoming HDCAs - for key, collection_pairs in inp_dataset_collections.items(): + for collection_pairs in inp_dataset_collections.values(): for collection, _ in collection_pairs: # if sub-collection mapping, this will be an DC not an HDCA # (e.g. part of collection not a collection instance) and thus won't have tags. From 6022fe7d7e90b0da0cad83589d5d630f6dd1d386 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 1 Feb 2018 21:03:02 -0500 Subject: [PATCH 3/3] Handle optional inputs when collecting preserved tags. https://github.com/galaxyproject/galaxy/pull/5403/files#r165283450 --- lib/galaxy/tools/actions/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index 98adf24defa..3d257c174e3 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -196,6 +196,9 @@ class DefaultToolAction(object): # grap tags from incoming HDAs preserved_tags = {} for data in inp_data.values(): + if not data: + continue + for tag in [t for t in data.tags if t.user_tname == 'name']: preserved_tags[tag.value] = tag