Merge pull request #8932 from mvdbeek/implicit_multiple_true_fix

[19.05] Fix implicitly converted datasets unused when mapping over

lib/galaxy/tools/__init__.py

@@ -52,6 +52,7 @@ from galaxy.tools.parameters.basic import (
     DataCollectionToolParameter,
     DataToolParameter,
     HiddenToolParameter,
+    ImplicitConversionRequired,
     SelectToolParameter,
     ToolParameter,
     workflow_building_modes,
@@ -2051,10 +2052,13 @@ class Tool(Dictifiable):
                     tool_dict['value'] = input.value_to_basic(state_inputs.get(input.name, initial_value), self.app, use_security=True)
                     tool_dict['default_value'] = input.value_to_basic(initial_value, self.app, use_security=True)
                     tool_dict['text_value'] = input.value_to_display_text(tool_dict['value'])
+                except ImplicitConversionRequired:
+                    tool_dict = input.to_dict(request_context)
+                    # This hack leads client to display a text field
+                    tool_dict['textable'] = True
                 except Exception:
                     tool_dict = input.to_dict(request_context)
                     log.exception("tools::to_json() - Skipping parameter expansion '%s'", input.name)
-                    pass
             if input_index >= len(group_inputs):
                 group_inputs.append(tool_dict)
             else:

lib/galaxy/tools/evaluation.py

@@ -188,7 +188,8 @@ class ToolEvaluator(object):
                                        dataset_paths=input_dataset_paths,
                                        datatypes_registry=self.app.datatypes_registry,
                                        tool=self.tool,
-                                       name=input.name)
+                                       name=input.name,
+                                       formats=input.formats)
 
             elif isinstance(input, DataToolParameter):
                 # FIXME: We're populating param_dict with conversions when

lib/galaxy/tools/parameters/basic.py

@@ -44,6 +44,10 @@ workflow_building_modes = Bunch(DISABLED=False, ENABLED=True, USE_HISTORY=1)
 WORKFLOW_PARAMETER_REGULAR_EXPRESSION = re.compile(r'\$\{.+?\}')
 
 
+class ImplicitConversionRequired(Exception):
+    pass
+
+
 def contains_workflow_parameter(value, search=False):
     if not isinstance(value, string_types):
         return False
@@ -848,7 +852,10 @@ class SelectToolParameter(ToolParameter):
             return self.legal_values
 
     def from_json(self, value, trans, other_values={}, require_legal_value=True):
-        legal_values = self.get_legal_values(trans, other_values)
+        try:
+            legal_values = self.get_legal_values(trans, other_values)
+        except ImplicitConversionRequired:
+            return value
         if (not legal_values or not require_legal_value) and is_runtime_context(trans, other_values):
             if self.multiple:
                 # While it is generally allowed that a select value can be '',
@@ -917,7 +924,10 @@ class SelectToolParameter(ToolParameter):
         return value
 
     def get_initial_value(self, trans, other_values):
-        options = list(self.get_options(trans, other_values))
+        try:
+            options = list(self.get_options(trans, other_values))
+        except ImplicitConversionRequired:
+            return None
         if not options:
             return None
         value = [optval for _, optval, selected in options if selected]
@@ -1222,6 +1232,13 @@ class ColumnListParameter(SelectToolParameter):
             # Use representative dataset if a dataset collection is parsed
             if isinstance(dataset, trans.app.model.HistoryDatasetCollectionAssociation):
                 dataset = dataset.to_hda_representative()
+            if isinstance(dataset, trans.app.model.HistoryDatasetAssociation) and self.ref_input and self.ref_input.formats:
+                target_ext, converted_dataset = dataset.find_conversion_destination(self.ref_input.formats)
+                if target_ext:
+                    if not converted_dataset:
+                        raise ImplicitConversionRequired
+                    else:
+                        dataset = converted_dataset
             # Columns can only be identified if the dataset is ready and metadata is available
             if not hasattr(dataset, 'metadata') or \
                     not hasattr(dataset.metadata, 'columns') or \

lib/galaxy/tools/parameters/dataset_matcher.py

@@ -109,7 +109,7 @@ class DatasetMatcher(object):
         self.filter_values = filter_values
 
     def valid_hda_match(self, hda, check_implicit_conversions=True):
-        """ Return False of this parameter can not be matched to the supplied
+        """ Return False if this parameter can not be matched to the supplied
         HDA, otherwise return a description of the match (either a
         HdaDirectMatch describing a direct match or a HdaImplicitMatch
         describing an implicit conversion.)

lib/galaxy/tools/parameters/wrapped.py

@@ -76,12 +76,14 @@ class WrappedParameters(object):
                                        dataset_instances,
                                        datatypes_registry=trans.app.datatypes_registry,
                                        tool=tool,
-                                       name=input.name)
+                                       name=input.name,
+                                       formats=input.formats)
             elif isinstance(input, DataToolParameter):
                 wrapper_kwds = dict(
                     datatypes_registry=trans.app.datatypes_registry,
                     tool=tool,
-                    name=input.name
+                    name=input.name,
+                    formats=input.formats
                 )
                 element_identifier = element_identifier_mapper.identifier(value, input_values)
                 if element_identifier:

lib/galaxy/tools/wrappers.py

@@ -234,7 +234,7 @@ class DatasetFilenameWrapper(ToolParameterValueWrapper):
         def items(self):
             return iter((k, self.get(k)) for k, v in self.metadata.items())
 
-    def __init__(self, dataset, datatypes_registry=None, tool=None, name=None, dataset_path=None, identifier=None):
+    def __init__(self, dataset, datatypes_registry=None, tool=None, name=None, dataset_path=None, identifier=None, formats=None):
         if not dataset:
             try:
                 # TODO: allow this to work when working with grouping
@@ -246,6 +246,10 @@ class DatasetFilenameWrapper(ToolParameterValueWrapper):
             # Tool wrappers should not normally be accessing .dataset directly,
             # so we will wrap it and keep the original around for file paths
             # Should we name this .value to maintain consistency with most other ToolParameterValueWrapper?
+            if formats:
+                target_ext, converted_dataset = dataset.find_conversion_destination(formats)
+                if target_ext and converted_dataset:
+                    dataset = converted_dataset
             self.unsanitized = dataset
             self.dataset = wrap_with_safe_string(dataset, no_wrap_classes=ToolParameterValueWrapper)
             self.metadata = self.MetadataWrapper(dataset.metadata)

test/api/test_tools.py

@@ -4,6 +4,7 @@ import json
 import os
 import tarfile
 
+import pytest
 from base import api
 from base import rules_test_data
 from base.populators import (
@@ -1922,6 +1923,44 @@ class ToolsTestCase(api.ApiTestCase):
             output_hdca = self.dataset_populator.get_history_collection_details(history_id, hid=implicit_collections[0]["hid"])
             assert output_hdca["collection_type"] == "list"
 
+    @skip_without_tool("column_multi_param")
+    def test_implicit_conversion_and_reduce(self):
+        with self.dataset_populator.test_history() as history_id:
+            self._run_implicit_collection_and_reduce(history_id=history_id, param="1")
+
+    @skip_without_tool("column_multi_param")
+    def test_implicit_conversion_and_reduce_invalid_param(self):
+        with self.dataset_populator.test_history() as history_id:
+            with pytest.raises(AssertionError):
+                self._run_implicit_collection_and_reduce(history_id=history_id, param="X")
+        details = self.dataset_populator.get_history_dataset_details(history_id=history_id, hid=3, assert_ok=False)
+        assert details['state'] == "error"
+        assert "parameter 'col': an invalid option" in details['misc_info']
+
+    def _run_implicit_collection_and_reduce(self, history_id, param):
+        fasta_path = self.test_data_resolver.get_filename("1.fasta")
+        with open(fasta_path, "r") as fasta_fh:
+            fasta_content = fasta_fh.read()
+            response = self.dataset_collection_populator.upload_collection(history_id, "list", elements=[
+                {
+                    "name": "test0",
+                    "src": "pasted",
+                    "paste_content": fasta_content,
+                    "ext": "fasta",
+                }
+            ])
+            self._assert_status_code_is(response, 200)
+            hdca_id = response.json()["outputs"][0]["id"]
+            inputs = {
+                "input1": {'src': 'hdca', 'id': hdca_id},
+                "col": param,
+            }
+            create = self._run("column_multi_param", history_id, inputs, assert_ok=True)
+            jobs = create['jobs']
+            self.assertEqual(len(jobs), 1)
+            content = self.dataset_populator.get_history_dataset_content(history_id, hid=3)
+            assert content.strip() == 'hg17', content
+
     @skip_without_tool("multi_data_repeat")
     def test_reduce_collections_in_repeat(self):
         with self.dataset_populator.test_history() as history_id:

test/functional/tools/column_multi_param.xml

@@ -5,7 +5,7 @@
     #end for#
   </command>
   <inputs>
-    <param type="data" format="tabular" name="input1" label="Input 1" multiple="true" />
+    <param name="input1" type="data" format="tabular" label="Input 1" multiple="true" />
     <param name="col" type="data_column" data_ref="input1" label="Column to Use" />
   </inputs>
   <outputs>