From 827ac9c0433f581166b5bd06b70a14a39f22ded2 Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 14 Nov 2017 16:30:18 -0500 Subject: [PATCH 1/5] Do not attempt to visit parameters of invalid conditionals --- lib/galaxy/tools/parameters/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 5c5fe29fa32..7742bfa13e2 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -102,8 +102,9 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre case_error = 'The selected case is unavailable/invalid.' pass callback_helper(input.test_param, values, new_name_prefix, label_prefix, parent_prefix=name_prefix, context=context, error=case_error) - values['__current_case__'] = input.get_current_case(values[input.test_param.name]) - visit_input_values(input.cases[values['__current_case__']].inputs, values, callback, new_name_prefix, label_prefix, parent_prefix=name_prefix, **payload) + if input.test_param.name in values: + values['__current_case__'] = input.get_current_case(values[input.test_param.name]) + visit_input_values(input.cases[values['__current_case__']].inputs, values, callback, new_name_prefix, label_prefix, parent_prefix=name_prefix, **payload) elif isinstance(input, Section): values = input_values[input.name] = input_values.get(input.name, {}) new_name_prefix = name_prefix + input.name + '|' From 1457fa7f937522d4825bb814a927a87b2e363cc8 Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 14 Nov 2017 18:49:36 -0500 Subject: [PATCH 2/5] Check for key/value errors only when testing conditional cases --- lib/galaxy/tools/parameters/__init__.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 7742bfa13e2..1509119ee6c 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -82,6 +82,12 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre if replace: input_values[input.name] = new_value + def get_current_case(input, input_values): + try: + return input.get_current_case(input_values[input.test_param.name]) + except (KeyError, ValueError) as e: + return -1 + context = ExpressionContext(input_values, context) payload = {'context': context, 'no_replacement_value': no_replacement_value} for input in inputs.values(): @@ -95,15 +101,10 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre elif isinstance(input, Conditional): values = input_values[input.name] = input_values.get(input.name, {}) new_name_prefix = name_prefix + input.name + '|' - case_error = None - try: - input.get_current_case(values[input.test_param.name]) - except: - case_error = 'The selected case is unavailable/invalid.' - pass + case_error = None if get_current_case(input, values) >= 0 else 'The selected case is unavailable/invalid.' callback_helper(input.test_param, values, new_name_prefix, label_prefix, parent_prefix=name_prefix, context=context, error=case_error) - if input.test_param.name in values: - values['__current_case__'] = input.get_current_case(values[input.test_param.name]) + values['__current_case__'] = get_current_case(input, values) + if values['__current_case__'] >= 0: visit_input_values(input.cases[values['__current_case__']].inputs, values, callback, new_name_prefix, label_prefix, parent_prefix=name_prefix, **payload) elif isinstance(input, Section): values = input_values[input.name] = input_values.get(input.name, {}) From 75f84bfc476ea94c22a38097d20f694265fcded6 Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 14 Nov 2017 20:18:10 -0500 Subject: [PATCH 3/5] Add test cases to cover version changes --- lib/galaxy/tools/parameters/__init__.py | 82 +++++++++++++++++++++---- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 1509119ee6c..351a1ba0b01 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -40,26 +40,84 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre >>> g = BooleanToolParameter( None, XML( '' ) ) >>> h = TextToolParameter( None, XML( '' ) ) >>> i = TextToolParameter( None, XML( '' ) ) - >>> b.name = 'b' + >>> j = TextToolParameter( None, XML( '' ) ) + >>> a.name = 'a' + >>> b.name = b.title = 'b' + >>> c.name = 'c' + >>> d.name = d.title = 'd' + >>> e.name = 'e' + >>> f.name = 'f' + >>> g.name = 'g' + >>> h.name = 'h' + >>> j.name = 'j' >>> b.inputs = odict([ ('c', c), ('d', d) ]) - >>> d.name = 'd' >>> d.inputs = odict([ ('e', e), ('f', f) ]) >>> f.test_param = g - >>> f.name = 'f' >>> f.cases = [ Bunch( value='true', inputs= { 'h': h } ), Bunch( value='false', inputs= { 'i': i } ) ] >>> - >>> def visitor( input, value, prefix, prefixed_name, **kwargs ): - ... print 'name=%s, prefix=%s, prefixed_name=%s, value=%s' % ( input.name, prefix, prefixed_name, value ) - >>> inputs = odict([('a',a),('b',b)]) + >>> def visitor( input, value, prefix, prefixed_name, prefixed_label, error, **kwargs ): + ... print 'name=%s, prefix=%s, prefixed_name=%s, prefixed_label=%s,value=%s' % ( input.name, prefix, prefixed_name, prefixed_label, value ) + ... if error: + ... print error + >>> inputs = odict([('a', a),('b', b)]) >>> nested = odict([ ('a', 1), ('b', [ odict([('c', 3), ( 'd', [odict([ ('e', 5), ('f', odict([ ('g', True), ('h', 7) ])) ]) ])]) ]) ]) >>> visit_input_values( inputs, nested, visitor ) - name=a, prefix=, prefixed_name=a, value=1 - name=c, prefix=b_0|, prefixed_name=b_0|c, value=3 - name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, value=5 - name=g, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|g, value=True - name=h, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|h, value=7 + name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 + name=c, prefix=b_0|, prefixed_name=b_0|c, prefixed_label=b 1 > c,value=3 + name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, prefixed_label=b 1 > d 1 > e,value=5 + name=g, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|g, prefixed_label=b 1 > d 1 > g,value=True + name=h, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|h, prefixed_label=b 1 > d 1 > h,value=7 >>> params_from_strings( inputs, params_to_strings( inputs, nested, None ), None )[ 'b' ][ 0 ][ 'd' ][ 0 ][ 'f' ][ 'g' ] is True True + + >>> # Conditional test parameter value does not match any case, warning is shown and child values are not visited + >>> f.test_param = j + >>> nested['b'][0]['d'][0]['f']['j'] = 'j' + >>> visit_input_values( inputs, nested, visitor ) + name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 + name=c, prefix=b_0|, prefixed_name=b_0|c, prefixed_label=b 1 > c,value=3 + name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, prefixed_label=b 1 > d 1 > e,value=5 + name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|j, prefixed_label=b 1 > d 1 > j,value=j + The selected case is unavailable/invalid. + + >>> # Conditional test parameter missing from state, value error + >>> del nested['b'][0]['d'][0]['f']['j'] + >>> visit_input_values( inputs, nested, visitor ) + name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 + name=c, prefix=b_0|, prefixed_name=b_0|c, prefixed_label=b 1 > c,value=3 + name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, prefixed_label=b 1 > d 1 > e,value=5 + name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|j, prefixed_label=b 1 > d 1 > j,value=None + No value found for 'b 1 > d 1 > j'. + + >>> # Conditional parameter missing from state, value error + >>> del nested['b'][0]['d'][0]['f'] + >>> visit_input_values( inputs, nested, visitor ) + name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 + name=c, prefix=b_0|, prefixed_name=b_0|c, prefixed_label=b 1 > c,value=3 + name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, prefixed_label=b 1 > d 1 > e,value=5 + name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|j, prefixed_label=b 1 > d 1 > j,value=None + No value found for 'b 1 > d 1 > j'. + + >>> # Conditional input name has changed due to version change, key error + >>> f.name = 'f_1' + >>> visit_input_values( inputs, nested, visitor ) + name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 + name=c, prefix=b_0|, prefixed_name=b_0|c, prefixed_label=b 1 > c,value=3 + name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, prefixed_label=b 1 > d 1 > e,value=5 + name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f_1|j, prefixed_label=b 1 > d 1 > j,value=None + No value found for 'b 1 > d 1 > j'. + + >>> # Other parameters are missing from state + >>> nested = odict([ ('b', [ odict([ ( 'd', [odict([ ('f', odict([ ('g', True), ('h', 7) ])) ]) ])]) ]) ]) + >>> visit_input_values( inputs, nested, visitor ) + name=a, prefix=, prefixed_name=a, prefixed_label=a,value=None + No value found for 'a'. + name=c, prefix=b_0|, prefixed_name=b_0|c, prefixed_label=b 1 > c,value=None + No value found for 'b 1 > c'. + name=e, prefix=b_0|d_0|, prefixed_name=b_0|d_0|e, prefixed_label=b 1 > d 1 > e,value=None + No value found for 'b 1 > d 1 > e'. + name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f_1|j, prefixed_label=b 1 > d 1 > j,value=None + No value found for 'b 1 > d 1 > j'. """ def callback_helper(input, input_values, name_prefix, label_prefix, parent_prefix, context=None, error=None): args = { @@ -85,7 +143,7 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre def get_current_case(input, input_values): try: return input.get_current_case(input_values[input.test_param.name]) - except (KeyError, ValueError) as e: + except (KeyError, ValueError): return -1 context = ExpressionContext(input_values, context) From 9e9edb7e338bcf6a3d4174fc0d8e420a5184ad6b Mon Sep 17 00:00:00 2001 From: guerler Date: Tue, 14 Nov 2017 20:47:50 -0500 Subject: [PATCH 4/5] Remove unnecessary name setting for non-group parameters --- lib/galaxy/tools/parameters/__init__.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 351a1ba0b01..24246f40d70 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -41,18 +41,12 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre >>> h = TextToolParameter( None, XML( '' ) ) >>> i = TextToolParameter( None, XML( '' ) ) >>> j = TextToolParameter( None, XML( '' ) ) - >>> a.name = 'a' >>> b.name = b.title = 'b' - >>> c.name = 'c' - >>> d.name = d.title = 'd' - >>> e.name = 'e' - >>> f.name = 'f' - >>> g.name = 'g' - >>> h.name = 'h' - >>> j.name = 'j' >>> b.inputs = odict([ ('c', c), ('d', d) ]) + >>> d.name = d.title = 'd' >>> d.inputs = odict([ ('e', e), ('f', f) ]) >>> f.test_param = g + >>> f.name = 'f' >>> f.cases = [ Bunch( value='true', inputs= { 'h': h } ), Bunch( value='false', inputs= { 'i': i } ) ] >>> >>> def visitor( input, value, prefix, prefixed_name, prefixed_label, error, **kwargs ): From 64eced315887f19b7efd6334d4a28f91e8fd6116 Mon Sep 17 00:00:00 2001 From: guerler Date: Wed, 15 Nov 2017 00:49:03 -0500 Subject: [PATCH 5/5] Fix comments --- lib/galaxy/tools/parameters/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 24246f40d70..d694d751efc 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -74,7 +74,7 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|j, prefixed_label=b 1 > d 1 > j,value=j The selected case is unavailable/invalid. - >>> # Conditional test parameter missing from state, value error + >>> # Test parameter missing in state, value error >>> del nested['b'][0]['d'][0]['f']['j'] >>> visit_input_values( inputs, nested, visitor ) name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 @@ -83,7 +83,7 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|j, prefixed_label=b 1 > d 1 > j,value=None No value found for 'b 1 > d 1 > j'. - >>> # Conditional parameter missing from state, value error + >>> # Conditional parameter missing in state, value error >>> del nested['b'][0]['d'][0]['f'] >>> visit_input_values( inputs, nested, visitor ) name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 @@ -92,7 +92,7 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f|j, prefixed_label=b 1 > d 1 > j,value=None No value found for 'b 1 > d 1 > j'. - >>> # Conditional input name has changed due to version change, key error + >>> # Conditional input name has changed e.g. due to tool changes, key error >>> f.name = 'f_1' >>> visit_input_values( inputs, nested, visitor ) name=a, prefix=, prefixed_name=a, prefixed_label=a,value=1 @@ -101,7 +101,7 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre name=j, prefix=b_0|d_0|, prefixed_name=b_0|d_0|f_1|j, prefixed_label=b 1 > d 1 > j,value=None No value found for 'b 1 > d 1 > j'. - >>> # Other parameters are missing from state + >>> # Other parameters are missing in state >>> nested = odict([ ('b', [ odict([ ( 'd', [odict([ ('f', odict([ ('g', True), ('h', 7) ])) ]) ])]) ]) ]) >>> visit_input_values( inputs, nested, visitor ) name=a, prefix=, prefixed_name=a, prefixed_label=a,value=None