From 179d597f82c32e2077dc0d03f2fd38ec6e38e9e6 Mon Sep 17 00:00:00 2001 From: James Taylor Date: Mon, 23 Mar 2009 08:54:27 -0400 Subject: [PATCH] Many improvements for parameter dependencies, particularly in workflow mode. In particular, allows late validation when running workflows if dependent on a dataset that will not be generated until later --- eggs.ini | 2 +- lib/galaxy/tools/__init__.py | 22 +++---- lib/galaxy/tools/parameters/__init__.py | 35 +++++++++++ lib/galaxy/tools/parameters/basic.py | 76 +++++++++++++++++------- lib/galaxy/web/controllers/workflow.py | 57 ++++++++---------- lib/galaxy/workflow/modules.py | 55 ++++++++++------- templates/workflow/editor_tool_form.mako | 22 ++++--- 7 files changed, 172 insertions(+), 97 deletions(-) diff --git a/eggs.ini b/eggs.ini index b744099beb0..3eb51773c7b 100644 --- a/eggs.ini +++ b/eggs.ini @@ -52,7 +52,7 @@ Babel = 0.9.4 ; extra version information [tags] -psycopg2 = _8.2.6_static +# psycopg2 = _8.2.6_static pysqlite = _3.5.4_static MySQL_python = _5.0.67_static python_lzo = _static diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 75238d16e2d..17f61c90efe 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -981,7 +981,7 @@ class Tool: # "dependent" parameter's value has not been reset ( dynamically generated based # on the new value of its dependency ) prior to reaching this point, so we need # to regenerate it before it is validated in check_param(). - incoming_value_generated = False + value_generated = False if not( 'runtool_btn' in incoming or 'URL' in incoming ): # Form must have been refreshed, probably due to a refresh_on_change try: @@ -995,25 +995,27 @@ class Tool: changed_params = {} changed_params[dependency_name] = dependency_value changed_params[input.name] = input - incoming_value = input.get_initial_value( trans, changed_params ) - incoming_value_generated = True + value = input.get_initial_value( trans, changed_params ) + error = None + value_generated = True # Delete the dependency_param from chagned_dependencies since its # dependent param has been generated based its new value. - del changed_dependencies[dependency_name] + ## Actually, don't do this. What if there is more than one dependent? + ## del changed_dependencies[dependency_name] break except: pass - if not incoming_value_generated: + if not value_generated: incoming_value = get_incoming_value( incoming, key, None ) - value, error = check_param( trans, input, incoming_value, context ) - # If a callback was provided, allow it to process the value - if item_callback: - old_value = state.get( input.name, None ) - value, error = item_callback( trans, key, input, value, error, old_value, context ) + value, error = check_param( trans, input, incoming_value, context ) if input.dependent_params and state[ input.name ] != value: # We need to keep track of changed dependency parametrs ( parameters # that have dependent parameters whose options are dynamically generated ) changed_dependencies[ input.name ] = value + # If a callback was provided, allow it to process the value + if item_callback: + old_value = state.get( input.name, None ) + value, error = item_callback( trans, key, input, value, error, old_value, context ) if error: errors[ input.name ] = error state[ input.name ] = value diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index a17fdcd5d5e..bf99080cd11 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -3,8 +3,43 @@ Classes encapsulating Galaxy tool parameters. """ from basic import * +from grouping import * from galaxy.util.json import * +def visit_input_values( inputs, input_values, callback, name_prefix="", label_prefix="" ): + """ + Given a tools parameter definition (`inputs`) and a specific set of + parameter `values`, call `callback` for each non-grouping parameter, + passing the parameter object, value, a constructed unique name, + and a display label. + + If the callback returns a value, it will be replace the old value. + + FIXME: There is redundancy between this and the visit_inputs methods of + Repeat and Group. This tracks labels and those do not. It would + be nice to unify all the places that recursively visit inputs. + """ + for input in inputs.itervalues(): + if isinstance( input, Repeat ): + for i, d in enumerate( input_values[ input.name ] ): + index = d['__index__'] + new_name_prefix = name_prefix + "%s_%d|" % ( input.name, index ) + new_label_prefix = label_prefix + "%s %d > " % ( input.title, i + 1 ) + visit_input_values( input.inputs, d, callback, new_name_prefix, new_label_prefix ) + elif isinstance( input, Conditional ): + values = input_values[ input.name ] + current = values["__current_case__"] + label_prefix = label_prefix + new_name_prefix = name_prefix + input.name + "|" + visit_input_values( input.cases[current].inputs, values, callback, new_name_prefix, label_prefix ) + else: + new_value = callback( input, + input_values[input.name], + prefixed_name = name_prefix + input.name, + prefixed_label = label_prefix + input.label ) + if new_value: + input_values[input.name] = new_value + def check_param( trans, param, incoming_value, param_values ): """ Check the value of a single parameter `param`. The value in diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 9eef2f3c12e..82fdcf67881 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -503,12 +503,11 @@ class SelectToolParameter( ToolParameter ): elif self.dynamic_options: return set( v for _, v, _ in eval( self.dynamic_options, self.tool.code_namespace, other_values ) ) else: - return self.legal_values - def get_html_field( self, trans=None, value=None, other_values={} ): + return self.legal_values + def get_html_field( self, trans=None, value=None, context={} ): # Dynamic options are not yet supported in workflow, allow # specifying the value as text for now. - if self.is_dynamic and trans.workflow_building_mode \ - and ( self.options is None or self.options.has_dataset_dependencies ): + if self.need_late_validation( trans, context ): assert isinstance( value, UnvalidatedValue ) value = value.value if self.multiple: @@ -522,7 +521,7 @@ class SelectToolParameter( ToolParameter ): if value is not None: if not isinstance( value, list ): value = [ value ] field = form_builder.SelectField( self.name, self.multiple, self.display, self.refresh_on_change ) - options = self.get_options( trans, other_values ) + options = self.get_options( trans, context ) for text, optval, selected in options: if isinstance( optval, UnvalidatedValue ): optval = optval.value @@ -531,11 +530,8 @@ class SelectToolParameter( ToolParameter ): selected = ( optval in value ) field.add_option( text, optval, selected ) return field - def from_html( self, value, trans=None, other_values={} ): - # HACK: trans may be None here if doing late validation, this is - # treated the same as not being in workflow mode - if self.is_dynamic and ( trans and trans.workflow_building_mode ) \ - and ( self.options is None or self.options.has_dataset_dependencies ): + def from_html( self, value, trans=None, context={} ): + if self.need_late_validation( trans, context ): if self.multiple: #While it is generally allowed that a select value can be '', #we do not allow this to be the case in a dynamically generated multiple select list being set in workflow building mode @@ -545,7 +541,7 @@ class SelectToolParameter( ToolParameter ): else: value = value.split( "\n" ) return UnvalidatedValue( value ) - legal_values = self.get_legal_values( trans, other_values ) + legal_values = self.get_legal_values( trans, context ) if isinstance( value, list ): if not(self.repeat): assert self.multiple, "Multiple values provided but parameter is not expecting multiple values" @@ -578,10 +574,39 @@ class SelectToolParameter( ToolParameter ): if isinstance( value, dict ) and value["__class__"] == "UnvalidatedValue": return UnvalidatedValue( value["value"] ) return super( SelectToolParameter, self ).value_from_basic( value, app ) + def need_late_validation( self, trans, context ): + """ + Determine whether we need to wait to validate this parameters value + given the current state. For parameters with static options this is + always false (can always validate immediately). For parameters with + dynamic options, we need to check whether the other parameters which + determine what options are valid have been set. For the old style + dynamic options which do not specify dependencies, this is always true + (must valiate at runtime). + """ + # Option list is statically defined, never need late validation + if not self.is_dynamic: + return False + # Old style dynamic options, no dependency information so there isn't + # a lot we can do: if we're dealing with workflows, have to assume + # late validation no matter what. + if self.dynamic_options is not None: + return ( trans is None or trans.workflow_building_mode ) + # If we got this far, we can actually look at the dependencies + # to see if their values will not be available until runtime. + for dep_name in self.get_dependencies(): + dep_value = context[ dep_name ] + # Dependency on a dataset that does not yet exist + if isinstance( dep_value, DummyDataset ): + return True + # Dependency on a value that does not yet exist + if isinstance( dep_value, RuntimeValue ): + return True + # Dynamic, but all dependenceis are known and have values + return False def get_initial_value( self, trans, context ): # More working around dynamic options for workflow - if self.is_dynamic and ( trans is None or trans.workflow_building_mode )\ - and ( self.options is None or self.options.has_dataset_dependencies ): + if self.need_late_validation( trans, context ): # Really the best we can do? return UnvalidatedValue( None ) options = list( self.get_options( trans, context ) ) @@ -750,7 +775,7 @@ class ColumnListParameter( SelectToolParameter ): return [ self.data_ref ] -class DrillDownSelectToolParameter( ToolParameter ): +class DrillDownSelectToolParameter( SelectToolParameter ): """ Parameter that takes on one (or many) of a specific set of values. Creating a hierarchical select menu, which allows users to 'drill down' a tree-like set of options. @@ -892,11 +917,11 @@ class DrillDownSelectToolParameter( ToolParameter ): Optionally attempt to retain the current value specific by 'value' """ return self.get_html_field( trans, value, other_values ).get_html() - + def get_html_field( self, trans=None, value=None, other_values={} ): # Dynamic options are not yet supported in workflow, allow # specifying the value as text for now. - if self.is_dynamic and trans.workflow_building_mode: + if self.need_late_validation( trans, other_values ): if value is not None: assert isinstance( value, UnvalidatedValue ) value = value.value @@ -911,7 +936,7 @@ class DrillDownSelectToolParameter( ToolParameter ): return form_builder.DrillDownField( self.name, self.multiple, self.display, self.refresh_on_change, self.get_options( trans, value, other_values ), value ) def from_html( self, value, trans=None, other_values={} ): - if self.is_dynamic and ( trans and trans.workflow_building_mode ): + if self.need_late_validation( trans, other_values ): if self.multiple: value = value.split( "\n" ) return UnvalidatedValue( value ) @@ -965,7 +990,7 @@ class DrillDownSelectToolParameter( ToolParameter ): initial_values.append( option['value'] ) recurse_options( initial_values, option['options'] ) # More working around dynamic options for workflow - if self.is_dynamic and trans.workflow_building_mode: + if self.need_late_validation( trans, other_values ): # Really the best we can do? return UnvalidatedValue( None ) initial_values = [] @@ -1002,12 +1027,15 @@ class DrillDownSelectToolParameter( ToolParameter ): for val in value: rval.append( get_option_display( val, self.options ) or val ) return "\n".join( rval ) + suffix + def get_dependencies( self ): """ Get the *names* of the other params this param depends on. """ return self.filtered.keys() +class DummyDataset( object ): + pass class DataToolParameter( ToolParameter ): # TODO, Nate: Make sure the following unit tests appropriately test the dataset security @@ -1042,8 +1070,9 @@ class DataToolParameter( ToolParameter ): self.multiple = str_bool( elem.get( 'multiple', False ) ) # Optional DataToolParameters are used in tools like GMAJ and LAJ self.optional = str_bool( elem.get( 'optional', False ) ) - #TODO: Enhance dynamic options for DataToolParameters - #Currently, only the special case key='build' of type='data_meta' is a valid filter + # TODO: Enhance dynamic options for DataToolParameters. Currently, + # only the special case key='build' of type='data_meta' is + # a valid filter options = elem.find( 'options' ) if options is None: self.options = None @@ -1121,7 +1150,7 @@ class DataToolParameter( ToolParameter ): """ # Can't look at history in workflow mode if trans.workflow_building_mode: - return None + return DummyDataset() assert trans is not None, "DataToolParameter requires a trans" history = trans.history assert history is not None, "DataToolParameter requires a history" @@ -1156,7 +1185,8 @@ class DataToolParameter( ToolParameter ): return '' def from_html( self, value, trans, other_values={} ): - # Can't look at history in workflow mode, skip validation and such + # Can't look at history in workflow mode, skip validation and such, + # although, this should never be called in workflow mode right? if trans.workflow_building_mode: return None if not value: @@ -1173,6 +1203,8 @@ class DataToolParameter( ToolParameter ): def to_string( self, value, app ): if value is None or isinstance( value, str ): return value + elif isinstance( value, DummyDataset ): + return None return value.id def to_python( self, value, app ): diff --git a/lib/galaxy/web/controllers/workflow.py b/lib/galaxy/web/controllers/workflow.py index bfbca2c506f..9123a650289 100644 --- a/lib/galaxy/web/controllers/workflow.py +++ b/lib/galaxy/web/controllers/workflow.py @@ -490,18 +490,23 @@ class WorkflowController( BaseController ): # If kwargs were provided, the states for each step should have # been POSTed for step in workflow.steps: + # Connections by input name + step.input_connections_by_name = \ + dict( ( conn.input_name, conn ) for conn in step.input_connections ) # Extract just the arguments for this step by prefix p = "%s|" % step.id l = len(p) step_args = dict( ( k[l:], v ) for ( k, v ) in kwargs.iteritems() if k.startswith( p ) ) step_errors = None if step.type == 'tool' or step.type is None: + module = module_factory.from_workflow_step( trans, step ) + # Any connected input needs to have value DummyDataset (these + # are not persisted so we need to do it every time) + module.add_dummy_datasets( connections=step.input_connections ) # Get the tool - tool = trans.app.toolbox.tools_by_id[ step.tool_id ] + tool = module.tool # Get the state - state = DefaultToolState() - state.decode( step_args.pop("tool_state"), tool, trans.app ) - step.state = state + step.state = state = module.state # Get old errors old_errors = state.inputs.pop( "__errors__", {} ) # Update the state @@ -512,38 +517,22 @@ class WorkflowController( BaseController ): state = step.state = module.decode_runtime_state( trans, step_args.pop( "tool_state" ) ) step_errors = module.update_runtime_state( trans, state, step_args ) if step_errors: - errors[step.id] = state.inputs["__errors__"] = step_errors - # Connections by input name - step.input_connections_by_name = dict( ( conn.input_name, conn ) for conn in step.input_connections ) + errors[step.id] = state.inputs["__errors__"] = step_errors if 'run_workflow' in kwargs and not errors: # Run each step, connecting outputs to inputs outputs = odict() - for step in workflow.steps: + for i, step in enumerate( workflow.steps ): if step.type == 'tool' or step.type is None: tool = trans.app.toolbox.tools_by_id[ step.tool_id ] input_values = step.state.inputs # Connect up - # TODO: Generalize out visitor - def visitor( inputs, input_values, prefix ): - for input in inputs.itervalues(): - if isinstance( input, Repeat ): - for i, d in enumerate( input_values[ input.name ] ): - index = d['__index__'] - new_prefix = prefix + "%s_%d|" % ( input.name, index ) - visitor( input.inputs, d, new_prefix) - elif isinstance( input, Conditional ): - values = input_values[ input.name ] - current = values["__current_case__"] - new_prefix = prefix + input.name + "|" - visitor( input.cases[current].inputs, values, new_prefix ) - else: - if isinstance( input, DataToolParameter ): - prefixed_name = prefix + input.name - if prefixed_name in step.input_connections_by_name: - conn = step.input_connections_by_name[ prefixed_name ] - input_values[ input.name ] = outputs[ conn.output_step.id ][ conn.output_name ] - visitor( tool.inputs, input_values, "" ) - # Execute it + def callback( input, value, prefixed_name, prefixed_label ): + if isinstance( input, DataToolParameter ): + if prefixed_name in step.input_connections_by_name: + conn = step.input_connections_by_name[ prefixed_name ] + input_values[ input.name ] = outputs[ conn.output_step.id ][ conn.output_name ] + visit_input_values( tool.inputs, step.state.inputs, callback ) + # Execute it outputs[ step.id ] = tool.execute( trans, step.state.inputs ) else: outputs[ step.id ] = step.module.execute( trans, step.state ) @@ -555,11 +544,13 @@ class WorkflowController( BaseController ): for step in workflow.steps: if step.type == 'tool' or step.type is None: # Restore the tool state for the step - tool = trans.app.toolbox.tools_by_id[ step.tool_id ] - state = DefaultToolState() - state.inputs = tool.params_from_strings( step.tool_inputs, trans.app ) + module = module_factory.from_workflow_step( trans, step ) + # Any connected input needs to have value DummyDataset (these + # are not persisted so we need to do it every time) + module.add_dummy_datasets( connections=step.input_connections ) # Store state with the step - step.state = state + step.module = module + step.state = module.state # Error dict if step.tool_errors: errors[step.id] = step.tool_errors diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 34d9f6ecce9..ba282746555 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -1,12 +1,13 @@ from elementtree.ElementTree import Element from galaxy import web -from galaxy.tools.parameters import DataToolParameter, RuntimeValue, check_param +from galaxy.tools.parameters import DataToolParameter, DummyDataset, RuntimeValue, check_param, visit_input_values from galaxy.tools import DefaultToolState from galaxy.tools.parameters.grouping import Repeat, Conditional from galaxy.util.bunch import Bunch from galaxy.util.json import from_json_string, to_json_string - + + class WorkflowModule( object ): def __init__( self, trans ): @@ -145,7 +146,7 @@ class InputDataModule( WorkflowModule ): def execute( self, trans, state ): return dict( output=state.inputs['input']) -class ToolModule( object ): +class ToolModule( WorkflowModule ): type = "tool" @@ -196,24 +197,13 @@ class ToolModule( object ): def get_data_inputs( self ): data_inputs = [] - def visitor( inputs, input_values, name_prefix, label_prefix ): - for input in inputs.itervalues(): - if isinstance( input, Repeat ): - for i, d in enumerate( input_values[ input.name ] ): - index = d['__index__'] - new_name_prefix = name_prefix + "%s_%d|" % ( input.name, index ) - new_label_prefix = label_prefix + "%s %d > " % ( input.title, i + 1 ) - visitor( input.inputs, d, new_name_prefix, new_label_prefix ) - elif isinstance( input, Conditional ): - values = input_values[ input.name ] - current = values["__current_case__"] - label_prefix = label_prefix - new_name_prefix = name_prefix + input.name + "|" - visitor( input.cases[current].inputs, values, new_name_prefix, label_prefix ) - else: - if isinstance( input, DataToolParameter ): - data_inputs.append( dict( name=name_prefix+input.name, label=label_prefix+input.label, extensions=input.extensions ) ) - visitor( self.tool.inputs, self.state.inputs, "", "" ) + def callback( input, value, prefixed_name, prefixed_label ): + if isinstance( input, DataToolParameter ): + data_inputs.append( dict( + name=prefixed_name, + label=prefixed_label, + extensions=input.extensions ) ) + visit_input_values( self.tool.inputs, self.state.inputs, callback ) return data_inputs def get_data_outputs( self ): data_outputs = [] @@ -221,15 +211,21 @@ class ToolModule( object ): data_outputs.append( dict( name=name, extension=format ) ) return data_outputs def get_config_form( self ): + self.add_dummy_datasets() return self.trans.fill_template( "workflow/editor_tool_form.mako", tool=self.tool, values=self.state.inputs, errors=( self.errors or {} ) ) def update_state( self, incoming ): # Build a callback that handles setting an input to be required at # runtime. We still process all other parameters the user might have - # set. + # set. We also need to make sure all datasets have a dummy value + # for dependencies to see make_runtime_key = incoming.get( 'make_runtime', None ) make_buildtime_key = incoming.get( 'make_buildtime', None ) def item_callback( trans, key, input, value, error, old_value, context ): + # Dummy value for Data parameters + if isinstance( input, DataToolParameter ): + return DummyDataset(), None + # Deal with build/runtime (does not apply to Data parameters) if key == make_buildtime_key: return input.get_initial_value( trans, context ), None elif isinstance( old_value, RuntimeValue ): @@ -241,6 +237,21 @@ class ToolModule( object ): # Update state using incoming values errors = self.tool.update_state( self.trans, self.tool.inputs, self.state.inputs, incoming, item_callback=item_callback ) self.errors = errors or None + def add_dummy_datasets( self, connections=None): + if connections: + # Store onnections by input name + input_connections_by_name = \ + dict( ( conn.input_name, conn ) for conn in connections ) + else: + input_connections_by_name = {} + # Any connected input needs to have value DummyDataset (these + # are not persisted so we need to do it every time) + def callback( input, value, prefixed_name, prefixed_label ): + if isinstance( input, DataToolParameter ): + if connections is None or prefixed_name in input_connections_by_name: + return DummyDataset() + visit_input_values( self.tool.inputs, self.state.inputs, callback ) + class WorkflowModuleFactory( object ): def __init__( self, module_types ): diff --git a/templates/workflow/editor_tool_form.mako b/templates/workflow/editor_tool_form.mako index d71379c480c..51b015b04f1 100644 --- a/templates/workflow/editor_tool_form.mako +++ b/templates/workflow/editor_tool_form.mako @@ -1,6 +1,10 @@ -<% from galaxy.tools.parameters import DataToolParameter, RuntimeValue %> +<% +from galaxy.tools.parameters import DataToolParameter, RuntimeValue +from galaxy.util.expressions import ExpressionContext +%> -<%def name="do_inputs( inputs, values, errors, prefix )"> +<%def name="do_inputs( inputs, values, errors, prefix, ctx=None )"> + <% ctx = ExpressionContext( values, ctx ) %> %for input_index, input in enumerate( inputs.itervalues() ): %if input.type == "repeat":
@@ -16,7 +20,7 @@ %>
${input.title} ${i + 1}
- ${do_inputs( input.inputs, repeat_values[ i ], rep_errors, prefix + input.name + "_" + str(index) + "|" )} + ${do_inputs( input.inputs, repeat_values[ i ], rep_errors, prefix + input.name + "_" + str(index) + "|", ctx )}
%endfor @@ -27,20 +31,20 @@ <% current_case = group_values['__current_case__'] %> <% group_prefix = prefix + input.name + "|" %> <% group_errors = errors.get( input.name, {} ) %> - ${row_for_param( input.test_param, group_values[ input.test_param.name ], group_errors, group_prefix, allow_runtime=False )} - ${do_inputs( input.cases[ current_case ].inputs, group_values, group_errors, group_prefix )} + ${row_for_param( input.test_param, group_values[ input.test_param.name ], group_errors, group_prefix, ctx, allow_runtime=False )} + ${do_inputs( input.cases[ current_case ].inputs, group_values, group_errors, group_prefix, ctx )} %else: %if input.name in values: - ${row_for_param( input, values[ input.name ], errors, prefix )} + ${row_for_param( input, values[ input.name ], errors, prefix, ctx )} %else: <% errors[ input.name ] = 'Value not stored, displaying default' %> - ${row_for_param( input, input.get_initial_value( trans, values ), errors, prefix )} + ${row_for_param( input, input.get_initial_value( trans, values ), errors, prefix, ctx )} %endif %endif %endfor -<%def name="row_for_param( param, value, error_dict, prefix, allow_runtime=True )"> +<%def name="row_for_param( param, value, error_dict, prefix, ctx, allow_runtime=True )"> %if error_dict.has_key( param.name ): <% cls = "form-row form-row-error" %> %else: @@ -77,7 +81,7 @@ %endif
- ${param.get_html_field( trans, value ).get_html( prefix )} + ${param.get_html_field( trans, value, ctx ).get_html( prefix )}
%endif %if error_dict.has_key( param.name ):