From 7ab5dc3d51edf1313ac74ac6ff07d26f507e7449 Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 18 May 2016 14:43:51 -0400 Subject: [PATCH 1/4] Remove asserts triggering false exceptions when dynamic parameters are evaluated for initial tool models --- lib/galaxy/tools/parameters/dynamic_options.py | 6 +----- test/unit/tools/test_select_parameters.py | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/tools/parameters/dynamic_options.py b/lib/galaxy/tools/parameters/dynamic_options.py index c70a385f27c..6e64e7cdef2 100644 --- a/lib/galaxy/tools/parameters/dynamic_options.py +++ b/lib/galaxy/tools/parameters/dynamic_options.py @@ -124,7 +124,6 @@ class DataMetaFilter( Filter ): if self.multiple: return dataset_value in file_value.split( self.separator ) return file_value == dataset_value - assert self.ref_name in other_values or ( trans is not None and trans.workflow_building_mode), "Required dependency '%s' not found in incoming values" % self.ref_name ref = other_values.get( self.ref_name, None ) is_data = isinstance( ref, galaxy.tools.wrappers.DatasetFilenameWrapper ) is_data_list = isinstance( ref, galaxy.tools.wrappers.DatasetListWrapper ) or isinstance( ref, list ) @@ -146,7 +145,7 @@ class DataMetaFilter( Filter ): else: meta_value = ref.metadata.get( self.key, None ) - if meta_value is None: # assert meta_value is not None, "Required metadata value '%s' not found in referenced dataset" % self.key + if meta_value is None: return [ ( disp_name, optval, selected ) for disp_name, optval, selected in options ] if self.column is not None: @@ -208,7 +207,6 @@ class ParamValueFilter( Filter ): def filter_options( self, options, trans, other_values ): if trans is not None and trans.workflow_building_mode: return [] - assert self.ref_name in other_values, "Required dependency '%s' not found in incoming values" % self.ref_name ref = other_values.get( self.ref_name, None ) for ref_attribute in self.ref_attribute: if not hasattr( ref, ref_attribute ): @@ -381,7 +379,6 @@ class RemoveValueFilter( Filter ): def filter_options( self, options, trans, other_values ): if trans is not None and trans.workflow_building_mode: return options - assert self.value is not None or ( self.ref_name is not None and self.ref_name in other_values ) or (self.meta_ref is not None and self.meta_ref in other_values ) or ( trans is not None and trans.workflow_building_mode), Exception( "Required dependency '%s' or '%s' not found in incoming values" % ( self.ref_name, self.meta_ref ) ) def compare_value( option_value, filter_value ): if isinstance( filter_value, list ): @@ -582,7 +579,6 @@ class DynamicOptions( object ): if self.dataset_ref_name: dataset = other_values.get( self.dataset_ref_name, None ) if not dataset or not hasattr( dataset, 'file_name' ): - log.warn( "Required dataset '%s' missing from input" % self.dataset_ref_name ) return [] # no valid dataset in history # Ensure parsing dynamic options does not consume more than a megabyte worth memory. path = dataset.file_name diff --git a/test/unit/tools/test_select_parameters.py b/test/unit/tools/test_select_parameters.py index 7eca591a0fa..fb6f4ccf763 100644 --- a/test/unit/tools/test_select_parameters.py +++ b/test/unit/tools/test_select_parameters.py @@ -20,8 +20,8 @@ class SelectToolParameterTestCase( BaseParameterTestCase ): self.options_xml = '''''' try: self.param.from_json("42", self.trans) - except AssertionError as err: - assert str(err) == "Required dependency 'input_bam' not found in incoming values" + except ValueError as err: + assert str(err) == "Parameter my_name requires a value, but has no legal values defined." return assert False From 1c537b4271b5e6b75c0df4fd0fb8e2b01d2de52a Mon Sep 17 00:00:00 2001 From: guerler Date: Thu, 19 May 2016 10:02:06 -0400 Subject: [PATCH 2/4] Remove conflicting raise/try/pass combo for select parameters --- lib/galaxy/tools/parameters/basic.py | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 6d6038f8191..91c9e9d78c4 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1001,15 +1001,7 @@ class SelectToolParameter( ToolParameter ): d = super( SelectToolParameter, self ).to_dict( trans ) # Get options, value. - options = [] - try: - options = self.get_options( trans, other_values ) - except AssertionError: - # we dont/cant set other_values (the {} above), so params that require other params to be filled will error: - # required dependency in filter_options - # associated DataToolParam in get_column_list - pass - + options = self.get_options( trans, other_values ) d[ 'options' ] = options if options: value = options[0][1] @@ -1191,10 +1183,8 @@ class ColumnListParameter( SelectToolParameter ): Generate a select list containing the columns of the associated dataset (if found). """ - # No value indicates a configuration error - assert self.data_ref in other_values, "Value for associated data reference not found (data_ref)." # Get the value of the associated data reference (a dataset) - dataset = other_values[ self.data_ref ] + dataset = other_values.get( self.data_ref, None ) # Check if a dataset is selected if not dataset: return [] @@ -1228,8 +1218,7 @@ class ColumnListParameter( SelectToolParameter ): """ options = [] if self.usecolnames: # read first row - assume is a header with metadata useful for making good choices - assert self.data_ref in other_values, "Value for associated data reference not found (data_ref)." - dataset = other_values[ self.data_ref ] + dataset = other_values.get( self.data_ref, None ) try: head = open( dataset.get_file_name(), 'r' ).readline() cnames = head.rstrip().split( '\t' ) @@ -1594,15 +1583,7 @@ class DrillDownSelectToolParameter( SelectToolParameter ): def to_dict( self, trans, view='collection', value_mapper=None, other_values={} ): # skip SelectToolParameter (the immediate parent) bc we need to get options in a different way here d = ToolParameter.to_dict( self, trans ) - - options = [] - try: - options = self.get_options( trans=trans, other_values=other_values ) - except KeyError: - # will sometimes error if self.is_dynamic and self.filtered - # bc we dont/cant fill out other_values above ({}) - pass - d['options'] = options + d['options'] = self.get_options( trans=trans, other_values=other_values ) d['display'] = self.display return d From 4893e0e58385f994327bbe819efc4a14e629f7cb Mon Sep 17 00:00:00 2001 From: guerler Date: Thu, 19 May 2016 10:14:39 -0400 Subject: [PATCH 3/4] Insert previously removed assertion into common validation pathway called through from_json --- lib/galaxy/tools/parameters/basic.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 91c9e9d78c4..996e9e398bc 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1245,6 +1245,8 @@ class ColumnListParameter( SelectToolParameter ): return SelectToolParameter.get_initial_value( self, trans, other_values ) def get_legal_values( self, trans, other_values ): + if self.data_ref not in other_values: + raise ValueError( "Value for associated data reference not found (data_ref)." ) return set( self.get_column_list( trans, other_values ) ) def get_dependencies( self ): From fa380a00cbb779f5b436e29cba2a466db59db62c Mon Sep 17 00:00:00 2001 From: guerler Date: Thu, 19 May 2016 10:49:58 -0400 Subject: [PATCH 4/4] Fix assertions for drilldown by raising properly displayed value errors instead --- lib/galaxy/tools/parameters/basic.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 996e9e398bc..cbbc91e7a99 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1480,10 +1480,11 @@ class DrillDownSelectToolParameter( SelectToolParameter ): return None if not isinstance( value, list ): value = [ value ] - if not( self.repeat ) and len( value ) > 1: - assert self.multiple, "Multiple values provided but parameter %s is not expecting multiple values" % self.name + if not self.repeat and len( value ) > 1 and not self.multiple: + raise ValueError( "Multiple values provided but parameter %s is not expecting multiple values." % self.name ) rval = [] - assert legal_values, "Parameter %s requires a value, but has no legal values defined" % self.name + if not legal_values: + raise ValueError( "Parameter %s requires a value, but has no legal values defined." % self.name ) for val in value: if val not in legal_values: raise ValueError( "An invalid option was selected for %s, %r, please verify" % ( self.name, val ) ) @@ -1520,9 +1521,8 @@ class DrillDownSelectToolParameter( SelectToolParameter ): for val in value: options = get_options_list( val ) rval.extend( options ) - if len( rval ) > 1: - if not self.repeat: - assert self.multiple, "Multiple values provided but parameter is not expecting multiple values" + if not self.repeat and len( rval ) > 1 and not self.multiple: + raise ValueError( "Multiple values provided but parameter %s is not expecting multiple values." % self.name ) rval = self.separator.join( map( value_map, rval ) ) if self.tool is None or self.tool.options.sanitize: if self.sanitizer: