merging heads

lib/galaxy/jobs/__init__.py

@@ -257,7 +257,7 @@ class JobQueue( object ):
                 return JOB_INPUT_DELETED
             # an error in the input data causes us to bail immediately
             elif idata.state == idata.states.ERROR:
-                job_wrapper.fail( "input data %d (file: %s) is in an error state" % ( idata.hid, idata.file_name ) )
+                job_wrapper.fail( "input data %d is in error state" % ( idata.hid ) )
                 return JOB_INPUT_ERROR
             elif idata.state != idata.states.OK:
                 # need to requeue
@@ -384,10 +384,17 @@ class JobWrapper( object ):
         job.refresh()
         # if the job was deleted, don't fail it
         if not job.state == model.Job.states.DELETED:
-            # If the failure is due to a Galaxy framework exception, save the traceback
-            # Do this first in case we generate a traceback below
+            # Check if the failure is due to an exception
             if exception:
+                # Save the traceback immediately in case we generate another
+                # below
                 job.traceback = traceback.format_exc()
+                # Get the exception and let the tool attempt to generate
+                # a better message
+                etype, evalue, tb =  sys.exc_info()
+                m = self.tool.handle_job_failure_exception( evalue )
+                if m:
+                    message = m
             if self.app.config.outputs_to_working_directory:
                 for dataset_path in self.get_output_fnames():
                     try:

lib/galaxy/tools/__init__.py

@@ -18,6 +18,7 @@ from galaxy import util, jobs, model
 from elementtree import ElementTree
 from parameters import *
 from parameters.grouping import *
+from parameters.validation import LateValidationError
 from galaxy.util.expressions import ExpressionContext
 from galaxy.tools.test import ToolTestBuilder
 from galaxy.tools.actions import DefaultToolAction
@@ -840,7 +841,8 @@ class Tool:
         return 'message.mako', dict( message_type='error', message='Your upload was interrupted.  If this was uninentional, please retry it.', refresh_frames=[], cont=None )
 
     def update_state( self, trans, inputs, state, incoming, prefix="", context=None,
-                      update_only=False, old_errors={}, changed_dependencies={} ):
+                      update_only=False, old_errors={}, changed_dependencies={},
+                      item_callback=None ):
         """
         Update the tool state in `state` using the user input in `incoming`. 
         This is designed to be called recursively: `inputs` contains the
@@ -894,7 +896,8 @@ class Tool:
                                                     context=context,
                                                     update_only=update_only,
                                                     old_errors=rep_old_errors,
-                                                    changed_dependencies=changed_dependencies )
+                                                    changed_dependencies=changed_dependencies,
+                                                    item_callback=item_callback )
                     if rep_errors:
                         any_group_errors = True
                         group_errors.append( rep_errors )
@@ -951,7 +954,8 @@ class Tool:
                                                       context=context,
                                                       update_only=update_only,
                                                       old_errors=group_old_errors,
-                                                      changed_dependencies=changed_dependencies )
+                                                      changed_dependencies=changed_dependencies,
+                                                      item_callback=item_callback )
                 if test_param_error:
                     group_errors[ input.test_param.name ] = test_param_error
                 if group_errors:
@@ -977,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:
@@ -991,21 +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 )
+                        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
@@ -1053,31 +1063,60 @@ class Tool:
             return
         self.handle_unvalidated_param_values_helper( self.inputs, input_values, app )
 
-    def handle_unvalidated_param_values_helper( self, inputs, input_values, app, context=None ):
+    def handle_unvalidated_param_values_helper( self, inputs, input_values, app, context=None, prefix="" ):
         """
         Recursive helper for `handle_unvalidated_param_values`
         """
         context = ExpressionContext( input_values, context )
         for input in inputs.itervalues():
             if isinstance( input, Repeat ):  
-                for d in input_values[ input.name ]:
-                    self.handle_unvalidated_param_values_helper( input.inputs, d, app, context )
+                for i, d in enumerate( input_values[ input.name ] ):
+                    rep_prefix = prefix + "%s %d > " % ( input.title, i + 1 )
+                    self.handle_unvalidated_param_values_helper( input.inputs, d, app, context, rep_prefix )
             elif isinstance( input, Conditional ):
                 values = input_values[ input.name ]
                 current = values["__current_case__"]
-                self.handle_unvalidated_param_values_helper( input.cases[current].inputs, values, app, context )
+                # NOTE: The test param doesn't need to be checked since
+                #       there would be no way to tell what case to use at
+                #       workflow build time. However I'm not sure if we are
+                #       actually preventing such a case explicately.
+                self.handle_unvalidated_param_values_helper( input.cases[current].inputs, values, app, context, prefix )
             else:
                 # Regular tool parameter
                 value = input_values[ input.name ]
                 if isinstance( value, UnvalidatedValue ):
-                    if value.value is None: #if value.value is None, it could not have been submited via html form and therefore .from_html can't be guaranteed to work
-                        value = None
-                    else:
-                        value = input.from_html( value.value, None, context )
-                    # Then do any further validation on the value
-                    input.validate( value, None )
+                    try:
+                        # Convert from html representation
+                        if value.value is None:
+                            # If value.value is None, it could not have been
+                            # submited via html form and therefore .from_html
+                            # can't be guaranteed to work
+                            value = None
+                        else:
+                            value = input.from_html( value.value, None, context )
+                        # Do any further validation on the value
+                        input.validate( value, None )
+                    except Exception, e:
+                        # Wrap an re-raise any generated error so we can
+                        # generate a more informative message
+                        v = input.value_to_display_text( value, self.app )
+                        message = "Failed runtime validation of %s%s (%s)" \
+                            % ( prefix, input.label, e )
+                        raise LateValidationError( message )
                     input_values[ input.name ] = value
     
+    def handle_job_failure_exception( self, e ):
+        """
+        Called by job.fail when an exception is generated to allow generation
+        of a better error message (returning None yields the default behavior)
+        """
+        message = None
+        # If the exception was generated by late validation, use its error
+        # message (contains the parameter name and value)
+        if isinstance( e, LateValidationError ):
+            message = e.message
+        return message
+    
     def build_param_dict( self, incoming, input_datasets, output_datasets, output_paths ):
         """
         Build the dictionary of parameters for substituting into the command

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 

lib/galaxy/tools/parameters/basic.py

@@ -94,12 +94,18 @@ class ToolParameter( object ):
         return value
         
     def value_to_basic( self, value, app ):
+        if isinstance( value, RuntimeValue ):
+            return { "__class__": "RuntimeValue" }
         return self.to_string( value, app )
         
     def value_from_basic( self, value, app, ignore_errors=False ):
         # HACK: Some things don't deal with unicode well, psycopg problem?
         if type( value ) == unicode:
             value = str( value )
+        # Handle Runtime values (valid for any parameter?)
+        if isinstance( value, dict ) and '__class__' in value and value['__class__'] == "RuntimeValue":
+            return RuntimeValue()
+        # Delegate to the 'to_python' method
         if ignore_errors:
             try:
                 return self.to_python( value, app )
@@ -497,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:
@@ -516,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
@@ -525,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
@@ -539,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"
@@ -567,16 +569,44 @@ class SelectToolParameter( ToolParameter ):
     def value_to_basic( self, value, app ):
         if isinstance( value, UnvalidatedValue ):
             return { "__class__": "UnvalidatedValue", "value": value.value }
-        return value
+        return super( SelectToolParameter, self ).value_to_basic( value, app )
     def value_from_basic( self, value, app, ignore_errors=False ):
-        if isinstance( value, dict ):
-            assert value["__class__"] == "UnvalidatedValue"
+        if isinstance( value, dict ) and value["__class__"] == "UnvalidatedValue":
             return UnvalidatedValue( value["value"] )
-        return 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 ) )
@@ -745,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.
@@ -887,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
@@ -906,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 )
@@ -953,15 +983,6 @@ class DrillDownSelectToolParameter( ToolParameter ):
                 assert self.multiple, "Multiple values provided but parameter is not expecting multiple values"
         return self.separator.join( rval )
     
-    def value_to_basic( self, value, app ):
-        if isinstance( value, UnvalidatedValue ):
-            return { "__class__": "UnvalidatedValue", "value": value.value }
-        return value
-    def value_from_basic( self, value, app, ignore_errors=False ):
-        if isinstance( value, dict ):
-            assert value["__class__"] == "UnvalidatedValue"
-            return UnvalidatedValue( value["value"] )
-        return value
     def get_initial_value( self, trans, context ):
         def recurse_options( initial_values, options ):
             for option in options:
@@ -969,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 = []
@@ -1006,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
@@ -1046,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
@@ -1125,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"
@@ -1160,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:
@@ -1174,25 +1200,19 @@ class DataToolParameter( ToolParameter ):
         else:
             return trans.app.model.HistoryDatasetAssociation.get( value )
 
-    def value_to_basic( self, value, app ):
+    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 value_from_basic( self, value, app, ignore_errors=False ):
-        """
-        Both of these values indicate that no dataset is selected.  However, 'None' 
-        indicates that the dataset is optional, while '' indicates that it is not.
-        """
+    def to_python( self, value, app ):
+        # Both of these values indicate that no dataset is selected.  However, 'None' 
+        # indicates that the dataset is optional, while '' indicates that it is not.
         if value is None or value == '' or value == 'None':
             return value
-        try:
-            return app.model.HistoryDatasetAssociation.get( int( value ) )
-        except:
-            if ignore_errors:
-                return value
-            else:
-                raise
+        return app.model.HistoryDatasetAssociation.get( int( value ) )
 
     def to_param_dict_string( self, value, other_values={} ):
         if value is None: return "None"
@@ -1294,6 +1314,14 @@ class UnvalidatedValue( object ):
     """
     def __init__( self, value ):
         self.value = value
+        
+class RuntimeValue( object ):
+    """
+    Wrapper to note a value that is not yet set, but will be required at
+    runtime.
+    """
+    pass
+    
 
 def str_bool(in_str):
     """

lib/galaxy/tools/parameters/validation.py

@@ -8,6 +8,10 @@ from galaxy import model
 
 log = logging.getLogger( __name__ )
 
+class LateValidationError( Exception ):
+    def __init__( self, message ):
+        self.message = message
+
 class Validator( object ):
     """
     A validator checks that a value meets some conditions OR raises ValueError

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 )    
-            if not errors:
+                    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 )
@@ -554,14 +543,15 @@ class WorkflowController( BaseController ):
         else:
             for step in workflow.steps:
                 if step.type == 'tool' or step.type is None:
-                    # Build a new 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 )
+                    # Restore the tool state for the step
+                    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
-                    # This should never actually happen since we don't allow
-                    # running workflows with errors (yet?)
+                    step.module = module
+                    step.state = module.state
+                    # Error dict
                     if step.tool_errors:
                         errors[step.id] = step.tool_errors
                 else:
@@ -575,7 +565,8 @@ class WorkflowController( BaseController ):
                     "workflow/run.mako", 
                     steps=workflow.steps,
                     workflow=stored,
-                    errors=errors )
+                    errors=errors,
+                    incoming=kwargs )
     
     @web.expose
     def configure_menu( self, trans, workflow_ids=None ):

lib/galaxy/workflow/modules.py

@@ -1,12 +1,13 @@
 from elementtree.ElementTree import Element
 
 from galaxy import web
-from galaxy.tools.parameters import DataToolParameter, 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,16 +211,46 @@ class ToolModule( object ):
             data_outputs.append( dict( name=name, extension=format ) )
         return data_outputs
     def get_config_form( self ):
-        def as_html( param, value, trans, prefix ):
-            if type( param ) is DataToolParameter:
-                return "Data input '" + param.name + "' (" + ( " or ".join( param.extensions ) ) + ")"
-            else:
-                return param.get_html_field( trans, value ).get_html( prefix )
+        self.add_dummy_datasets()
         return self.trans.fill_template( "workflow/editor_tool_form.mako", 
-            tool=self.tool, as_html=as_html, values=self.state.inputs, errors=( self.errors or {} ) )
-    def update_state( self, incoming ):
-        errors = self.tool.update_state( self.trans, self.tool.inputs, self.state.inputs, incoming )
-        self.errors = errors or None
+            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. 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 ):
+                return old_value, None
+            elif key == make_runtime_key:
+                return RuntimeValue(), None
+            else:
+                return value, error
+        # 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 ):

static/june_2007_style/base.css.tmpl

@@ -144,6 +144,7 @@ div.repeat-group-item
 {
     border-left: solid $form_border 5px;
     margin-left: 10px;
+    margin-bottom: 10px;
 }
 
 div.form-row-error

static/june_2007_style/blue/base.css

@@ -144,6 +144,7 @@ div.repeat-group-item
 {
     border-left: solid #d8b365 5px;
     margin-left: 10px;
+    margin-bottom: 10px;
 }
 
 div.form-row-error

static/scripts/galaxy.base.js

@@ -65,9 +65,14 @@ function make_popupmenu( button_element, options ) {
     var click = function( e ) {
         var o = $(button_element).offset();
         $("#popup-helper").mousedown( clean ).show();
-        $( menu_element ).click( clean ).css( { top: -1000 } ).show().css( {
+        // Show off screen to get size right
+        $( menu_element ).click( clean ).css( { left: 0, top: -1000 } ).show();
+        console.log( e.pageX, $(document).scrollLeft() + $(window).width(), $(menu_element).width() );
+        var x = Math.min( e.pageX - 2, $(document).scrollLeft() + $(window).width() - $(menu_element).width() - 5 );
+            
+        $( menu_element ).css( {
             top: e.pageY - 2,
-            left: e.pageX - 2 // + $(button_element).width() - $(menu_element).width()
+            left: x
         } );
         return false;
     };

static/scripts/galaxy.panels.js

@@ -194,30 +194,7 @@ function show_modal( title, body, buttons, extra_buttons ) {
     }
 };
     
-// Popup -- is this up to date?
-
-function make_popupmenu( button_element, options ) {
-    var menu_element = $( "<div class='popupmenu'><div class='popupmenu-top'><div class='popupmenu-top-inner'/></div></div>" ).appendTo( "body" );
-    $.each( options, function( k, v ) {
-        $( "<div class='popupmenu-item' />" ).html( k ).click( v ).appendTo( menu_element );
-    });
-    var clean = function() {
-        $(menu_element).unbind().hide();
-        $("#popup-helper").unbind().hide();
-    };
-    var click = function() {
-        var o = $(button_element).offset();
-        $("#popup-helper").mousedown( clean ).show();
-        $( menu_element ).click( clean ).css( { top: -1000 } ).show().css( {
-            top: o.top + $(button_element).height() + 9,
-            left: o.left + $(button_element).width() - $(menu_element).width()
-        } );
-    };
-    $( button_element ).click( click );
-};
-
 // Tab management
-
 	
 $(function() {
     $("span.tab").each( function() {

templates/workflow/editor.mako

@@ -21,6 +21,7 @@
         ensure_dd_helper();
         make_left_panel( $("#left"), $("#center"), $("#left-border" ) );
         make_right_panel( $("#right"), $("#center"), $("#right-border" ) );
+        ensure_popup_helper();
         ## handle_minwidth_hint = rp.handle_minwidth_hint;
     </script>
 </%def>
@@ -39,6 +40,7 @@
     <script type='text/javascript' src="${h.url_for('/static/scripts/jquery.form.js')}"> </script>
     <script type='text/javascript' src="${h.url_for('/static/scripts/jquery.json.js')}"> </script>
 
+    <script type='text/javascript' src="${h.url_for('/static/scripts/galaxy.base.js')}"> </script>
     <script type='text/javascript' src="${h.url_for('/static/scripts/galaxy.workflow_editor.canvas.js')}"> </script>
     
     <!--[if lt IE 7]>
@@ -212,14 +214,7 @@
     };
 
     function show_form_for_tool( text, node ) {
-        // $("#overlay, #modalwrapper" ).show();
-        //$("#modal iframe").attr( 'src', "${h.url_for( action='tool_form' )}?tool_id=" + tool_id ).load( function () {
-        //    $("#modalloadwrapper").hide();
-        //});
-        // $("#right-content").load( "${h.url_for( action='tool_form' )}", { tool_id: tool_id }, function () {
-        //     
-        // });
-        $("#right-content").html( text );
+        $("#right-content").html( text );    
         $("#right-content").find( "form" ).ajaxForm( {
             type: 'POST',
             dataType: 'json',
@@ -239,6 +234,20 @@
             $(this).find( "select[refresh_on_change='true']").change( function() {
                 $(form).submit();
             });
+            $(this).find( ".popupmenu" ).each( function() {
+                var b = $('<a class="popup-arrow">&#9660;</a>')
+                var options = {}
+                $(this).find( "button" ).each( function() {
+                    var name = $(this).attr( 'name' )
+                    var value = $(this).attr( 'value' );
+                    options[ $(this).text() ] = function() {
+                        $(form).append( "<input type='hidden' name='"+name+"' value='"+value+"' />" ).submit();
+                    }
+                });
+                b.insertAfter( this );
+                $(this).remove();
+                make_popupmenu( b, options );
+            });
         });
     }
     

templates/workflow/editor_tool_form.mako

@@ -1,4 +1,10 @@
-<%def name="do_inputs( inputs, values, errors, prefix )">
+<%
+from galaxy.tools.parameters import DataToolParameter, RuntimeValue
+from galaxy.util.expressions import ExpressionContext
+%>
+
+<%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":
       <div class="repeat-group">
@@ -14,7 +20,7 @@
             %>
             <div class="repeat-group-item">
             <div class="form-title-row"><b>${input.title} ${i + 1}</b></div>
-            ${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 )}
             <div class="form-row"><input type="submit" name="${prefix}${input.name}_${index}_remove" value="Remove ${input.title} ${i+1}"></div>
             </div>
           %endfor
@@ -25,34 +31,64 @@
       <% 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 )}
-      ${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>
 
-<%def name="row_for_param( param, value, error_dict, prefix )">
+<%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:
         <% cls = "form-row" %>
     %endif
     <div class="${cls}">
-        <label>${param.get_label()}</label>
-        <div>
-            ${as_html( param, value, t, prefix )}
-        </div>
-        %if error_dict.has_key( param.name ):
-        <div style="color: red; font-weight: bold; padding-top: 1px; padding-bottom: 3px;">
-            <div style="width: 300px;"><img style="vertical-align: middle;" src="${h.url_for('/static/style/error_small.png')}">&nbsp;<span style="vertical-align: middle;">${error_dict[param.name]}</span></div>
-        </div>
+        ## Data parameters are very special since their value / runtime state
+        ## comes from connectors
+        %if type( param ) is DataToolParameter:
+            <label>
+                ${param.get_label()}
+            </label>
+            <div>
+                Data input '${param.name}' (${" or ".join( param.extensions )})
+            </div>
+        %else:
+            %if isinstance( value, RuntimeValue ):    
+                <label>
+                    ${param.get_label()}
+                    <span class="popupmenu">
+                        <button type="submit" name="make_buildtime" value="${prefix}${param.name}">Set in advance</button>
+                    </span>
+                </label>
+                <div>
+                    <i>To be set at runtime</i>
+                </div>
+            %else:
+                <label>
+                    ${param.get_label()}
+                    %if allow_runtime:
+                        <span class="popupmenu">
+                            <button type="submit" name="make_runtime" value="${prefix}${param.name}">Set at runtime</button>
+                        </span>
+                    %endif
+                </label>
+                <div>
+                    ${param.get_html_field( trans, value, ctx ).get_html( prefix )}          
+                </div>
+            %endif
+            %if error_dict.has_key( param.name ):
+            <div style="color: red; font-weight: bold; padding-top: 1px; padding-bottom: 3px;">
+                <div style="width: 300px;"><img style="vertical-align: middle;" src="${h.url_for('/static/style/error_small.png')}">&nbsp;<span style="vertical-align: middle;">${error_dict[param.name]}</span></div>
+            </div>
+            %endif
         %endif
         <div style="clear: both"></div>       
     </div>

templates/workflow/run.mako

@@ -1,19 +1,28 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
-<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<%inherit file="/base.mako"/>
 
-<head>
-<link href="${h.url_for('/static/style/base.css')}" rel="stylesheet" type="text/css" />
-<style type="text/css">
-div.toolForm{
-    margin-top: 10px;
-    margin-bottom: 10px;
-}
-</style>
-</head>
+<%def name="javascripts()">
+    ${parent.javascripts()}
+    <script type="text/javascript">        
+        $( function() {
+            $( "select[@refresh_on_change='true']").change( function() {
+                $( "#tool_form" ).submit();
+            });
+        });
+    </script>
+</%def>
+
+<%def name="stylesheets()">
+    <link href="${h.url_for('/static/style/base.css')}" rel="stylesheet" type="text/css" />
+    <style type="text/css">
+    div.toolForm{
+        margin-top: 10px;
+        margin-bottom: 10px;
+    }
+    </style>
+</%def>
 
 <%
-from galaxy.tools.parameters import DataToolParameter
+from galaxy.tools.parameters import DataToolParameter, RuntimeValue
 %>
 
 <%def name="do_inputs( inputs, values, errors, prefix, step, other_values = None )">
@@ -70,9 +79,28 @@ from galaxy.tools.parameters import DataToolParameter
                     %>
                     Output dataset '${conn.output_name}' from step ${int(conn.output_step.order_index)+1}
                 %else:
-                    ${param.get_html_field( t, dict(), other_values ).get_html( str(step.id) + "|" + prefix )}
+                    ## FIXME: Initialize in the controller
+                    <%
+                    if value is None:
+                        value = other_values[ param.name ] = param.get_initial_value( t, other_values )
+                    %>
+                    ${param.get_html_field( t, value, other_values ).get_html( str(step.id) + "|" + prefix )}
                     <input type="hidden" name="${step.id}|__force_update__${prefix}${param.name}" value="true" />
                 %endif
+            %elif isinstance( value, RuntimeValue ) or \
+                  ( str(step.id) + '|__runtime__' + prefix + param.name ) in incoming:
+                ## On the first load we may see a RuntimeValue, so we write
+                ## an input field using the initial value for the param.
+                ## Subsequents posts will no longer have the runtime value
+                ## (since an actualy value will be posted) so we add a hidden
+                ## field so we know to continue drawing form for this param.
+                ## FIXME: This logic shouldn't be in the template. The
+                ## controller should go through the inputs on the first
+                ## load, fill in initial values where needed, and mark
+                ## all that are runtime modifiable in some way.
+                <% value = other_values[ param.name ] = param.get_initial_value( t, other_values ) %>
+                ${param.get_html_field( t, value, other_values ).get_html( str(step.id) + "|" + prefix )}
+                <input type="hidden" name="${step.id}|__runtime__${prefix}${param.name}" value="true" />
             %else:
                 ${param.value_to_display_text( value, app )}
             %endif
@@ -86,32 +114,30 @@ from galaxy.tools.parameters import DataToolParameter
     </div>
 </%def>
 
-<body>
-    <h2>Running workflow "${workflow.name}"</h2>
+<h2>Running workflow "${workflow.name}"</h2>
 
-    <form method="POST">
-    ## <input type="hidden" name="workflow_name" value="${workflow.name | h}" />
-    %for i, step in enumerate( steps ):    
-        %if step.type == 'tool' or step.type is None:
-          <% tool = app.toolbox.tools_by_id[step.tool_id] %>
-          <input type="hidden" name="${step.id}|tool_state" value="${step.state.encode( tool, app )}">
-          <div class="toolForm">
-              <div class="toolFormTitle">Step ${int(step.order_index)+1}: ${tool.name}</div>
-              <div class="toolFormBody">
-                  ${do_inputs( tool.inputs, step.state.inputs, errors.get( step.id, dict() ), "", step )}
-              </div>
+<form id="tool_form" name="tool_form" method="POST">
+## <input type="hidden" name="workflow_name" value="${workflow.name | h}" />
+%for i, step in enumerate( steps ):    
+    %if step.type == 'tool' or step.type is None:
+      <% tool = app.toolbox.tools_by_id[step.tool_id] %>
+      <input type="hidden" name="${step.id}|tool_state" value="${step.state.encode( tool, app )}">
+      <div class="toolForm">
+          <div class="toolFormTitle">Step ${int(step.order_index)+1}: ${tool.name}</div>
+          <div class="toolFormBody">
+              ${do_inputs( tool.inputs, step.state.inputs, errors.get( step.id, dict() ), "", step )}
           </div>
-        %else:
-        <% module = step.module %>
-          <input type="hidden" name="${step.id}|tool_state" value="${module.encode_runtime_state( t, step.state )}">
-          <div class="toolForm">
-              <div class="toolFormTitle">Step ${int(step.order_index)+1}: ${module.name}</div>
-              <div class="toolFormBody">
-                  ${do_inputs( module.get_runtime_inputs(), step.state.inputs, errors.get( step.id, dict() ), "", step )}
-              </div>
+      </div>
+    %else:
+    <% module = step.module %>
+      <input type="hidden" name="${step.id}|tool_state" value="${module.encode_runtime_state( t, step.state )}">
+      <div class="toolForm">
+          <div class="toolFormTitle">Step ${int(step.order_index)+1}: ${module.name}</div>
+          <div class="toolFormBody">
+              ${do_inputs( module.get_runtime_inputs(), step.state.inputs, errors.get( step.id, dict() ), "", step )}
           </div>
-        %endif
-    %endfor
-    <input type="submit" value="Run workflow" />
-    </form>
-</body>
+      </div>
+    %endif
+%endfor
+<input type="submit" name="run_workflow" value="Run workflow" />
+</form>