Merge pull request #10438 from bernt-matthias/backport/topic/compare_tests

[20.09] backport #10365: add test for data_column parameter referring to non-tabular data (bed) and fix bug
This commit is contained in:
Marius van den Beek
2020-10-17 10:46:41 +02:00
committed by GitHub
11 changed files with 74 additions and 47 deletions
+1 -1
View File
@@ -639,7 +639,7 @@ class Data(metaclass=DataMeta):
return datatypes_registry.get_converters_by_datatype(original_dataset.ext)
def find_conversion_destination(self, dataset, accepted_formats, datatypes_registry, **kwd):
"""Returns ( target_ext, existing converted dataset )"""
"""Returns ( direct_match, converted_ext, existing converted dataset )"""
return datatypes_registry.find_conversion_destination_for_dataset_by_extensions(dataset, accepted_formats, **kwd)
def convert_dataset(self, trans, original_dataset, target_type, return_output=False, visible=True, deps=None, target_context=None, history=None):
@@ -90,7 +90,9 @@ class DisplayApplicationDataParameter(DisplayApplicationParameter):
rval = data.get_converted_files_by_type(ext)
if rval:
return rval
assert data.find_conversion_destination(self.formats)[0] is not None, "No conversion path found for data param: %s" % self.name
direct_match, target_ext, converted_dataset = data.find_conversion_destination(self.formats)
assert direct_match or target_ext is not None, "No conversion path found for data param: %s" % self.name
return None
return data
@@ -108,19 +110,20 @@ class DisplayApplicationDataParameter(DisplayApplicationParameter):
# start conversion
# FIXME: Much of this is copied (more than once...); should be some abstract method elsewhere called from here
# find target ext
target_ext, converted_dataset = data.find_conversion_destination(self.formats, converter_safe=True)
if target_ext and not converted_dataset:
if isinstance(data, DisplayDataValueWrapper):
data = data.value
new_data = next(iter(data.datatype.convert_dataset(trans, data, target_ext, return_output=True, visible=False).values()))
new_data.hid = data.hid
new_data.name = data.name
trans.sa_session.add(new_data)
assoc = trans.app.model.ImplicitlyConvertedDatasetAssociation(parent=data, file_type=target_ext, dataset=new_data, metadata_safe=False)
trans.sa_session.add(assoc)
trans.sa_session.flush()
elif converted_dataset and converted_dataset.state == converted_dataset.states.ERROR:
raise Exception("Dataset conversion failed for data parameter: %s" % self.name)
direct_match, target_ext, converted_dataset = data.find_conversion_destination(self.formats, converter_safe=True)
if not direct_match:
if target_ext and not converted_dataset:
if isinstance(data, DisplayDataValueWrapper):
data = data.value
new_data = next(iter(data.datatype.convert_dataset(trans, data, target_ext, return_output=True, visible=False).values()))
new_data.hid = data.hid
new_data.name = data.name
trans.sa_session.add(new_data)
assoc = trans.app.model.ImplicitlyConvertedDatasetAssociation(parent=data, file_type=target_ext, dataset=new_data, metadata_safe=False)
trans.sa_session.add(assoc)
trans.sa_session.flush()
elif converted_dataset and converted_dataset.state == converted_dataset.states.ERROR:
raise Exception("Dataset conversion failed for data parameter: %s" % self.name)
return self.get_value(other_values, dataset_hash, user_hash, trans)
def is_preparing(self, other_values):
+10 -3
View File
@@ -883,7 +883,11 @@ class Registry:
return None
def find_conversion_destination_for_dataset_by_extensions(self, dataset_or_ext, accepted_formats, converter_safe=True):
"""Returns ( target_ext, existing converted dataset )"""
"""
returns (direct_match, converted_ext, converted_dataset)
- direct match is True iff no the data set already has an accepted format
- target_ext becomes None if conversion is not possible (or necesary)
"""
if hasattr(dataset_or_ext, "ext"):
ext = dataset_or_ext.ext
dataset = dataset_or_ext
@@ -891,6 +895,9 @@ class Registry:
ext = dataset_or_ext
dataset = None
if self.get_datatype_by_extension(ext) is not None and self.get_datatype_by_extension(ext).matches_any(accepted_formats):
return True, None, None
for convert_ext in self.get_converters_by_datatype(ext):
convert_ext_datatype = self.get_datatype_by_extension(convert_ext)
if convert_ext_datatype is None:
@@ -903,8 +910,8 @@ class Registry:
continue
else:
ret_data = None
return (convert_ext, ret_data)
return (None, None)
return False, convert_ext, ret_data
return False, None, None
def get_composite_extensions(self):
return [ext for (ext, d_type) in self.datatypes_by_extension.items() if d_type.composite_type is not None]
+9 -9
View File
@@ -85,15 +85,15 @@ class DefaultToolAction:
return None
if formats is None:
formats = input.formats
if not data.datatype.matches_any(formats):
# Need to refresh in case this conversion just took place, i.e. input above in tool performed the same conversion
trans.sa_session.refresh(data)
target_ext, converted_dataset = data.find_conversion_destination(formats)
if target_ext:
if converted_dataset:
data = converted_dataset
else:
data = data.get_converted_dataset(trans, target_ext, target_context=parent, history=history)
# Need to refresh in case this conversion just took place, i.e. input above in tool performed the same conversion
trans.sa_session.refresh(data)
direct_match, target_ext, converted_dataset = data.find_conversion_destination(formats)
if not direct_match and target_ext:
if converted_dataset:
data = converted_dataset
else:
data = data.get_converted_dataset(trans, target_ext, target_context=parent, history=history)
input_name = prefix + input.name
# Checked security of whole collection all at once if mapping over this input, else
+2 -2
View File
@@ -195,10 +195,10 @@ class ToolEvaluator:
for conversion_name, conversion_extensions, conversion_datatypes in input.conversions:
# If we are at building cmdline step, then converters
# have already executed
conv_ext, converted_dataset = input_values[input.name].find_conversion_destination(conversion_datatypes)
direct_match, conv_ext, converted_dataset = input_values[input.name].find_conversion_destination(conversion_datatypes)
# When dealing with optional inputs, we'll provide a
# valid extension to be used for None converted dataset
if not conv_ext:
if not direct_match and not conv_ext:
conv_ext = conversion_extensions[0]
# input_values[ input.name ] is None when optional
# dataset, 'conversion' of optional dataset should
+2 -2
View File
@@ -1262,8 +1262,8 @@ class ColumnListParameter(SelectToolParameter):
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:
direct_match, target_ext, converted_dataset = dataset.find_conversion_destination(self.ref_input.formats)
if not direct_match and target_ext:
if not converted_dataset:
raise ImplicitConversionRequired
else:
@@ -116,12 +116,12 @@ class DatasetMatcher:
"""
rval = False
formats = self.param.formats
if self.dataset_matcher_factory.matches_any_format(hda.extension, formats):
direct_match, target_ext, converted_dataset = hda.find_conversion_destination(formats)
if direct_match:
rval = HdaDirectMatch(hda)
else:
if not check_implicit_conversions:
return False
target_ext, converted_dataset = hda.find_conversion_destination(formats)
if target_ext:
original_hda = hda
if converted_dataset:
@@ -219,11 +219,10 @@ class SummaryDatasetCollectionMatcher:
formats = self.dataset_matcher.param.formats
uses_implicit_conversion = False
for extension in extensions:
if self.dataset_matcher_factory.matches_any_format(extension, formats):
continue
datatypes_registry = self._trans.app.datatypes_registry
converted_ext, _ = datatypes_registry.find_conversion_destination_for_dataset_by_extensions(extension, formats)
direct_match, converted_ext, _ = datatypes_registry.find_conversion_destination_for_dataset_by_extensions(extension, formats)
if direct_match:
continue
if not converted_ext:
return False
else:
+2 -2
View File
@@ -264,8 +264,8 @@ class DatasetFilenameWrapper(ToolParameterValueWrapper):
# 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:
direct_match, target_ext, converted_dataset = dataset.find_conversion_destination(formats)
if not direct_match and target_ext and converted_dataset:
dataset = converted_dataset
self.unsanitized = dataset
self.dataset = wrap_with_safe_string(dataset, no_wrap_classes=ToolParameterValueWrapper)
+16
View File
@@ -30,5 +30,21 @@ echo "col_names $col_names" >> '$output2'
</assert_contents>
</output>
</test>
<!-- test if non tabular data also creates entries by using the default
value (which is the 1st column, but empty if filling the options fails) -->
<test>
<param name="input1" value="1.bed" />
<output name="output1">
<assert_contents>
<has_line line="chr1" />
</assert_contents>
</output>
<output name="output2">
<assert_contents>
<has_line line="col 1" />
<has_line line="col_names 1" />
</assert_contents>
</output>
</test>
</tests>
</tool>
+6 -5
View File
@@ -39,6 +39,7 @@ class DataToolParameterTestCase(BaseParameterTestCase):
assert field['options']['hda'][1]['name'] == "hda1"
hda2.extension = 'data'
hda2.conversion_destination = (False, None, None)
field = self._simple_field()
assert len(field['options']['hda']) == 1, field
assert field['options']['hda'][0]['name'] == "hda1"
@@ -66,7 +67,7 @@ class DataToolParameterTestCase(BaseParameterTestCase):
def test_field_implicit_conversion_new(self):
hda1 = MockHistoryDatasetAssociation(name="hda1", id=1)
hda1.extension = 'data'
hda1.conversion_destination = ("tabular", None)
hda1.conversion_destination = (False, "tabular", None)
self.stub_active_datasets(hda1)
field = self._simple_field()
assert len(field['options']['hda']) == 1
@@ -76,7 +77,7 @@ class DataToolParameterTestCase(BaseParameterTestCase):
def test_field_implicit_conversion_existing(self):
hda1 = MockHistoryDatasetAssociation(name="hda1", id=1)
hda1.extension = 'data'
hda1.conversion_destination = ("tabular", MockHistoryDatasetAssociation(name="hda1converted", id=2))
hda1.conversion_destination = (False, "tabular", MockHistoryDatasetAssociation(name="hda1converted", id=2))
self.stub_active_datasets(hda1)
field = self._simple_field()
assert len(field['options']['hda']) == 1
@@ -125,14 +126,14 @@ class DataToolParameterTestCase(BaseParameterTestCase):
hda1 = MockHistoryDatasetAssociation(name="hda1", id=1)
hda1.extension = 'data'
converted = MockHistoryDatasetAssociation(name="hda1converted", id=2)
hda1.conversion_destination = ("tabular", converted)
hda1.conversion_destination = (False, "tabular", converted)
self.stub_active_datasets(hda1)
assert converted == self.param.get_initial_value(self.trans, {})
def test_get_initial_with_to_be_converted_data(self):
hda1 = MockHistoryDatasetAssociation(name="hda1", id=1)
hda1.extension = 'data'
hda1.conversion_destination = ("tabular", None)
hda1.conversion_destination = (False, "tabular", None)
self.stub_active_datasets(hda1)
assert hda1 == self.param.get_initial_value(self.trans, {}), hda1
@@ -190,7 +191,7 @@ class MockHistoryDatasetAssociation(object):
self.deleted = False
self.dataset = test_dataset
self.visible = True
self.conversion_destination = (None, None)
self.conversion_destination = (True, None, None)
self.extension = "txt"
self.dbkey = "hg19"
self.implicitly_converted_parent_datasets = False
+4 -3
View File
@@ -23,6 +23,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp):
# Datasets that don't match datatype are not valid.
self.mock_hda.visible = True
self.mock_hda.extension = 'data'
self.mock_hda.conversion_destination = (False, None, None)
assert not self.test_context.hda_match(self.mock_hda)
def test_valid_hda_direct_match(self):
@@ -42,7 +43,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp):
# dataset.
self.mock_hda.extension = 'data'
converted_hda = model.HistoryDatasetAssociation()
self.mock_hda.conversion_destination = ("tabular", converted_hda)
self.mock_hda.conversion_destination = (False, "tabular", converted_hda)
hda_match = self.test_context.hda_match(self.mock_hda)
assert hda_match
@@ -54,7 +55,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp):
# Find conversion returns a target extension to convert to, but not
# a previously implicitly converted dataset.
self.mock_hda.extension = 'data'
self.mock_hda.conversion_destination = ("tabular", None)
self.mock_hda.conversion_destination = (False, "tabular", None)
hda_match = self.test_context.hda_match(self.mock_hda)
assert hda_match
@@ -64,7 +65,7 @@ class DatasetMatcherTestCase(TestCase, UsesApp):
def test_hda_match_properly_skips_conversion(self):
self.mock_hda.extension = 'data'
self.mock_hda.conversion_destination = ("tabular", bunch.Bunch())
self.mock_hda.conversion_destination = (False, "tabular", bunch.Bunch())
hda_match = self.test_context.hda_match(self.mock_hda, check_implicit_conversions=False)
assert not hda_match