From 27800b4e3353762147baae502780c7597164d3dd Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 15 Oct 2019 09:51:15 -0400 Subject: [PATCH 01/16] Richer workflow inputs. - Allow parameter inputs to be optional. - Allow parameter inputs to have default values (if optional). - Allow data and collection parameter to be optional. - Unimplemented but new abstractions should allow having users specify extensions manually if they'd like for data and collection parameters. The previous workflow Module interface would more or less persist Galaxy's "Tool State" directly into the database. The tool state framework persists state in an un-typed fashion (e.g. the optional value for data inputs would be 'True' instead of True). The Format 2 YAML that we would like to publish to Galaxy and export from Galaxy would much more cleanly represent this as actual boolean values. The problem gets even worse with parameter inputs where we want the default value to be settable only if optional is True. This requires a conditional in the "Tool Form" language - so the persisted value would be nested structures without typing. For instance - instead of ``{default: 5, parameter_type: "integer", "optional": true}`` that we might want to store into the database or read from a workflow definition file, the framework would require something like ``{"parameter_type": "integer", "parameter_defintion": {"optional": {"optional": "True", {"specify_optional": {"specify_default": "True", "default": "5"}}}}}``. My preference may have been to have plugins defined in the client that just produce the minimal JSON - but I didn't know how to implement that and Sam's preference was that the client use the tool form framework for everything on the side panel anyway though. As a result, this commit adds abstractions to separate module tool state (for commiunicating with client and tied to tool form rendering) from abstract export state - loaded from the API when not coming from the tool and stored in database. To summarize workflow module changes... - module.get_inputs can build complex tool form parameteras (unchanged) - module.state is always the tool form state (unchanged) - module.get_export_state is added to get the abstract state for export and for reasoning about internally - module.recover_state now assumes recovery from abstract step state until from_tool_form=True is supplied - API entry points tied to the tool form (build_module) just pass through from_tool_form by default. - The client now sends from_tool_form into the API whenever it serializes the workflow state for re-saving --- .../scripts/mvc/tool/tool-form-composite.js | 3 + .../scripts/mvc/workflow/workflow-view.js | 3 +- lib/galaxy/managers/workflows.py | 16 +- lib/galaxy/model/__init__.py | 11 + lib/galaxy/tools/parameters/__init__.py | 7 +- lib/galaxy/webapps/base/controller.py | 3 +- lib/galaxy/webapps/galaxy/api/workflows.py | 8 +- lib/galaxy/workflow/modules.py | 282 ++++++++++++++++-- lib/galaxy/workflow/run.py | 10 +- lib/galaxy/workflow/run_request.py | 13 +- lib/galaxy_test/api/test_workflows.py | 118 +++++++- lib/galaxy_test/base/populators.py | 3 + lib/galaxy_test/base/workflow_fixtures.py | 138 +++++++++ 13 files changed, 567 insertions(+), 48 deletions(-) diff --git a/client/galaxy/scripts/mvc/tool/tool-form-composite.js b/client/galaxy/scripts/mvc/tool/tool-form-composite.js index fe1512fee99f..f1f97d78a79a 100644 --- a/client/galaxy/scripts/mvc/tool/tool-form-composite.js +++ b/client/galaxy/scripts/mvc/tool/tool-form-composite.js @@ -598,6 +598,9 @@ var View = Backbone.View.extend({ if (!input_def.step_linked) { if (this._isDataStep(step)) { validated = input_value && input_value.values && input_value.values.length > 0; + if (!validated && input_def.optional) { + validated = true; + } } else { validated = input_def.optional || diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view.js b/client/galaxy/scripts/mvc/workflow/workflow-view.js index 3538ba432aa4..644300625488 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-view.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-view.js @@ -689,7 +689,7 @@ export default Backbone.View.extend({ Utils.request({ url: `${getAppRoot()}api/workflows/${self.options.id}`, type: "PUT", - data: { workflow: self.workflow.to_simple() }, + data: { workflow: self.workflow.to_simple(), from_tool_form: true }, success: function(data) { var body = $("
").text(data.message); if (data.errors) { @@ -743,6 +743,7 @@ export default Backbone.View.extend({ data: { workflow_name: rename_name, workflow_annotation: rename_annotation, + from_tool_form: true, workflow_data: function() { return JSON.stringify(self.workflow.to_simple()); } diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 3d8862eb18a2..5980cd8e0e2f 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -300,6 +300,7 @@ def build_workflow_from_raw_description( create_stored_workflow=True, exact_tools=True, fill_defaults=False, + from_tool_form=False, ): data = raw_workflow_description.as_dict # Put parameters in workflow mode @@ -315,6 +316,7 @@ def build_workflow_from_raw_description( name=name, exact_tools=exact_tools, fill_defaults=fill_defaults, + from_tool_form=from_tool_form, ) if 'uuid' in data: workflow.uuid = data['uuid'] @@ -624,7 +626,7 @@ def _workflow_to_dict_editor(self, trans, stored, workflow, tooltip=True, is_sub 'label': module.label, 'content_id': module.get_content_id(), 'name': module.get_name(), - 'tool_state': module.get_state(), + 'tool_state': module.get_tool_state(), 'errors': module.get_errors(), 'inputs': module.get_all_inputs(connectable_only=True), 'outputs': module.get_all_outputs(), @@ -823,10 +825,7 @@ def _workflow_to_dict_export(self, trans, stored=None, workflow=None): annotation_str = self.get_item_annotation_str(trans.sa_session, trans.user, step) or '' content_id = module.get_content_id() # Export differences for backward compatibility - if module.type == 'tool': - tool_state = module.get_state(nested=False) - else: - tool_state = module.state.inputs + tool_state = module.get_export_state() # Step info step_dict = { 'id': step.order_index, @@ -885,9 +884,10 @@ def _workflow_to_dict_export(self, trans, stored=None, workflow=None): # Data inputs, legacy section not used anywhere within core input_dicts = [] step_state = module.state.inputs or {} - if "name" in step_state and module.type != 'tool': - name = step_state.get("name") - input_dicts.append({"name": name, "description": annotation_str}) + if module.type != 'tool': + name = step_state.get("name") or module.label + if name: + input_dicts.append({"name": name, "description": annotation_str}) for name, val in step_state.items(): input_type = type(val) if input_type == RuntimeValue: diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 1f1f3f2eb70c..d9e61dbd973e 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4658,6 +4658,15 @@ def __init__(self): def tool_uuid(self): return self.dynamic_tool and self.dynamic_tool.uuid + @property + def input_default_value(self): + tool_inputs = self.tool_inputs + tool_state = tool_inputs + default_value = tool_state.get("default") + if default_value: + default_value = json.loads(default_value)["value"] + return default_value + def get_input(self, input_name): for step_input in self.inputs: if step_input.name == input_name: @@ -5115,6 +5124,8 @@ def to_dict(self, view='collection', value_mapper=None, step_details=False, lega if output_step_type in ['data_input', 'data_collection_input']: src = "hda" if output_step_type == 'data_input' else 'hdca' for job_input in job.input_datasets: + if not job_input.dataset: + continue if job_input.name == step_input.input_name: inputs[str(step_input.output_step.order_index)] = { "id": job_input.dataset_id, "src": src, diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 206b62311afa..4ecccc583cd7 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -19,7 +19,7 @@ __all__ = ('DataCollectionToolParameter', 'DataToolParameter', 'SelectToolParameter') -def visit_input_values(inputs, input_values, callback, name_prefix='', label_prefix='', parent_prefix='', context=None, no_replacement_value=REPLACE_ON_TRUTHY): +def visit_input_values(inputs, input_values, callback, name_prefix='', label_prefix='', parent_prefix='', context=None, no_replacement_value=REPLACE_ON_TRUTHY, replace_optional_connections=False): """ Given a tools parameter definition (`inputs`) and a specific set of parameter `values`, call `callback` for each non-grouping parameter, @@ -116,10 +116,11 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre 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): + value = input_values.get(input.name) args = { 'input' : input, 'parent' : input_values, - 'value' : input_values.get(input.name), + 'value' : value, 'prefixed_name' : '%s%s' % (name_prefix, input.name), 'prefixed_label' : '%s%s' % (label_prefix, input.label or input.name), 'prefix' : parent_prefix, @@ -135,6 +136,8 @@ def callback_helper(input, input_values, name_prefix, label_prefix, parent_prefi replace = new_value != no_replacement_value if replace: input_values[input.name] = new_value + elif replace_optional_connections and is_runtime_value(value): + input_values[input.name] = input.value def get_current_case(input, input_values): try: diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py index f08c3b81d0b4..b0d7e8b5b3f8 100644 --- a/lib/galaxy/webapps/base/controller.py +++ b/lib/galaxy/webapps/base/controller.py @@ -1249,7 +1249,7 @@ def _import_shared_workflow(self, trans, stored): session.flush() return imported_stored - def _workflow_from_dict(self, trans, data, source=None, add_to_menu=False, publish=False, exact_tools=True, fill_defaults=False): + def _workflow_from_dict(self, trans, data, source=None, add_to_menu=False, publish=False, exact_tools=True, fill_defaults=False, from_tool_form=False): """ Creates a workflow from a dict. Created workflow is stored in the database and returned. """ @@ -1264,6 +1264,7 @@ def _workflow_from_dict(self, trans, data, source=None, add_to_menu=False, publi publish=publish, exact_tools=exact_tools, fill_defaults=fill_defaults, + from_tool_form=from_tool_form, ) return created_workflow.stored_workflow, created_workflow.missing_tools diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 79aed89c023d..68f47a11942e 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -551,6 +551,7 @@ def update(self, trans, id, payload, **kwds): annotation defaults to existing annotation * menu_entry optional boolean marking if the workflow should appear in the user's menu, if not present, workflow menu entries are not modified + * from_tool_form True iff encoded state coming in is encoded for the tool form. :rtype: dict :returns: serialized version of the workflow @@ -613,11 +614,11 @@ def build_module(self, trans, payload={}): """ inputs = payload.get('inputs', {}) trans.workflow_building_mode = workflow_building_modes.ENABLED - module = module_factory.from_dict(trans, payload) + module = module_factory.from_dict(trans, payload, from_tool_form=True) if 'tool_state' not in payload: module_state = {} populate_state(trans, module.get_inputs(), inputs, module_state, check=False) - module.recover_state(module_state) + module.recover_state(module_state, from_tool_form=True) return { 'label' : inputs.get('__label', ''), 'annotation' : inputs.get('__annotation', ''), @@ -719,10 +720,11 @@ def __import_or_update_kwds(self, payload): # Fill in missing tool state for hand built so the workflow can run, default of this # should become True at some point in the future I imagine. fill_defaults = util.string_as_bool(payload.get("fill_defaults", False)) - + from_tool_form = payload.get("from_tool_form", False) return { 'exact_tools': exact_tools, 'fill_defaults': fill_defaults, + 'from_tool_form': from_tool_form, } def __normalize_workflow(self, trans, as_dict): diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 57c6f2102e1d..83fd3c20589b 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -38,7 +38,9 @@ ConnectedValue, DataCollectionToolParameter, DataToolParameter, + FloatToolParameter, HiddenToolParameter, + IntegerToolParameter, is_runtime_value, parameter_types, runtime_to_json, @@ -46,6 +48,10 @@ TextToolParameter, workflow_building_modes ) +from galaxy.tools.parameters.grouping import ( + Conditional, + ConditionalWhen, +) from galaxy.tools.parameters.history_query import HistoryQuery from galaxy.tools.parameters.wrapped import make_dict_copy from galaxy.util import unicodify @@ -94,7 +100,7 @@ def from_dict(Class, trans, d, **kwds): @classmethod def from_workflow_step(Class, trans, step, **kwds): module = Class(trans, **kwds) - module.recover_state(step.tool_inputs) + module.recover_state(step.tool_inputs, from_tool_form=False) module.label = step.label return module @@ -136,13 +142,31 @@ def get_state(self, nested=True): else: return self.state.inputs + def get_export_state(self): + return self.get_state(nested=True) + + def get_tool_state(self): + return self.get_state(nested=False) + def recover_state(self, state, **kwds): - """ Recover state `dict` from simple dictionary describing configuration - state (potentially from persisted step state). + """Recover tool state (as self.state and self.state.inputs) from dictionary describing + configuration state (potentially from persisted step state). Sub-classes should supply a `default_state` method which contains the initial state `dict` with key, value pairs for all available attributes. + + Input parameter state may be a typed dictionary or a tool state dictionary generated by + the web client (if from_tool_form in kwds is True). + + If classes need to distinguish between typed clean dictionary representations + of state and the state generated for tool form (with extra nesting logic in the + state for conditionals, un-typed string parameters, etc...) they should + override the step_state_to_tool_state method to make this transformation. """ + from_tool_form = kwds.get("from_tool_form", False) + if not from_tool_form: + state = self.step_state_to_tool_state(state) + self.state = DefaultToolState() inputs = self.get_inputs() if inputs: @@ -150,6 +174,9 @@ def recover_state(self, state, **kwds): else: self.state.inputs = safe_loads(state) or {} + def step_state_to_tool_state(self, state): + return state + def get_errors(self, **kwargs): """ This returns a step related error message as string or None """ return None @@ -554,7 +581,13 @@ def to_dict(self, *args, **kwds): return as_dict +def optional_param(optional): + optional_value = BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", checked=optional)) + return optional_value + + class InputModule(WorkflowModule): + default_optional = False def get_runtime_state(self): state = DefaultToolState() @@ -596,6 +629,26 @@ def execute(self, trans, progress, invocation_step, use_cached_job=False): def recover_mapping(self, invocation_step, progress): progress.set_outputs_for_input(invocation_step, already_persisted=True) + def get_export_state(self): + return self._parse_state_into_dict() + + def _parse_state_into_dict(self): + inputs = self.state.inputs + rval = {} + if "optional" in inputs: + optional = bool(inputs["optional"]) + else: + optional = self.default_optional + rval["optional"] = optional + return rval + + def step_state_to_tool_state(self, state): + return state + + def save_to_step(self, step): + step.type = self.type + step.tool_inputs = self._parse_state_into_dict() + class InputDataModule(InputModule): type = "data_input" @@ -616,7 +669,19 @@ def get_filter_set(self, connections=None): return ', '.join(filter_set) def get_runtime_inputs(self, connections=None): - return dict(input=DataToolParameter(None, Element("param", name="input", label=self.label, multiple=False, type="data", format=self.get_filter_set(connections)), self.trans)) + parameter_def = self._parse_state_into_dict() + optional = parameter_def["optional"] + # TODO: extension from parameter_def + input_format = self.get_filter_set(connections) + input_param = DataToolParameter(None, Element("param", name="input", label=self.label, multiple=False, type="data", format=input_format, optional=str(optional)), self.trans) + return dict(input=input_param) + + def get_inputs(self): + parameter_def = self._parse_state_into_dict() + optional = parameter_def["optional"] + inputs = OrderedDict() + inputs["optional"] = optional_param(optional) + return inputs class InputDataCollectionModule(InputModule): @@ -626,7 +691,10 @@ class InputDataCollectionModule(InputModule): collection_type = default_collection_type def get_inputs(self): - collection_type = self.state.inputs.get("collection_type", self.default_collection_type) + parameter_def = self._parse_state_into_dict() + collection_type = parameter_def["collection_type"] + optional = parameter_def["optional"] + # TODO: extensions... input_collection_type = TextToolParameter(None, XML( ''' @@ -635,12 +703,19 @@ def get_inputs(self): ''' % collection_type)) - return dict(collection_type=input_collection_type) + + inputs = OrderedDict() + inputs["collection_type"] = input_collection_type + inputs["optional"] = optional_param(optional) + return inputs def get_runtime_inputs(self, **kwds): - collection_type = self.state.inputs.get("collection_type", self.default_collection_type) - input_element = Element("param", name="input", label=self.label, type="data_collection", collection_type=collection_type) - return dict(input=DataCollectionToolParameter(None, input_element, self.trans)) + parameter_def = self._parse_state_into_dict() + collection_type = parameter_def["collection_type"] + optional = parameter_def["optional"] + input_element = Element("param", name="input", label=self.label, type="data_collection", collection_type=collection_type, optional=str(optional)) + input_param = DataCollectionToolParameter(None, input_element, self.trans) + return dict(input=input_param) def get_all_outputs(self, data_only=False): return [ @@ -652,50 +727,151 @@ def get_all_outputs(self, data_only=False): ) ] + def _parse_state_into_dict(self): + state_as_dict = super(InputDataCollectionModule, self)._parse_state_into_dict() + inputs = self.state.inputs + if "collection_type" in inputs: + collection_type = inputs["collection_type"] + else: + collection_type = self.default_collection_type + state_as_dict["collection_type"] = collection_type + return state_as_dict + class InputParameterModule(WorkflowModule): type = "parameter_input" name = "Input parameter" default_parameter_type = "text" default_optional = False + default_default_value = '' parameter_type = default_parameter_type optional = default_optional + default_value = default_default_value def get_inputs(self): - # TODO: Use an external xml or yaml file to load the parameter definition - parameter_type = self.state.inputs.get("parameter_type", self.default_parameter_type) - optional = self.state.inputs.get("optional", self.default_optional) + parameter_def = self._parse_state_into_dict() + parameter_type = parameter_def["parameter_type"] + optional = parameter_def["optional"] input_parameter_type = SelectToolParameter(None, XML( ''' - + - ''')) + ''' % parameter_type)) for i, option in enumerate(input_parameter_type.static_options): option = list(option) if option[1] == parameter_type: # item 0 is option description, item 1 is value, item 2 is "selected" option[2] = True input_parameter_type.static_options[i] = tuple(option) - return OrderedDict([("parameter_type", input_parameter_type), - ("optional", BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", value=optional)))]) + + parameter_type_cond = Conditional() + parameter_type_cond.name = "parameter_definition" + parameter_type_cond.test_param = input_parameter_type + cases = [] + + for param_type in ["text", "integer", "float"]: + if param_type == "text": + if parameter_type == "text": + default = parameter_def.get("default") or "" + else: + default = "" + input_default_value = TextToolParameter(None, XML( + ''' + + + ''' + % default)) + elif param_type == "integer": + if parameter_type == "integer": + default = str(parameter_def.get("default") or "0") + else: + default = "0" + input_default_value = IntegerToolParameter(None, XML( + ''' + + + ''' + % default)) + elif param_type == "float": + if parameter_type == "float": + default = str(parameter_def.get("default") or "0.0") + else: + default = "0.0" + input_default_value = FloatToolParameter(None, XML( + ''' + + + ''' + % default)) + # color parameter defaults? + optional_value = BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", checked=optional)) + optional_cond = Conditional() + optional_cond.name = "optional" + optional_cond.test_param = optional_value + + when_text = ConditionalWhen() + when_text.value = param_type + when_text.inputs = OrderedDict() + when_text.inputs["optional"] = optional_cond + + specify_default_checked = "default" in parameter_def + specify_default = BooleanToolParameter(None, Element("param", name="specify_default", label="Specify a default value", type="boolean", checked=specify_default_checked)) + specify_default_cond = Conditional() + specify_default_cond.name = "specify_default" + specify_default_cond.test_param = specify_default + + when_specify_default_true = ConditionalWhen() + when_specify_default_true.value = "true" + when_specify_default_true.inputs = OrderedDict() + when_specify_default_true.inputs["default"] = input_default_value + + when_specify_default_false = ConditionalWhen() + when_specify_default_false.value = "false" + when_specify_default_false.inputs = OrderedDict() + + specify_default_cond_cases = [when_specify_default_true, when_specify_default_false] + specify_default_cond.cases = specify_default_cond_cases + + when_true = ConditionalWhen() + when_true.value = "true" + when_true.inputs = OrderedDict() + when_true.inputs["default"] = specify_default_cond + + when_false = ConditionalWhen() + when_false.value = "false" + when_false.inputs = OrderedDict() + + optional_cases = [when_true, when_false] + optional_cond.cases = optional_cases + + cases.append(when_text) + + parameter_type_cond.cases = cases + return OrderedDict([("parameter_definition", parameter_type_cond)]) def get_runtime_inputs(self, **kwds): - parameter_type = self.state.inputs.get("parameter_type", self.default_parameter_type) - optional = self.state.inputs.get("optional", self.default_optional) + parameter_def = self._parse_state_into_dict() + parameter_type = parameter_def["parameter_type"] + optional = parameter_def["optional"] if parameter_type not in ["text", "boolean", "integer", "float", "color"]: raise ValueError("Invalid parameter type for workflow parameters encountered.") parameter_class = parameter_types[parameter_type] parameter_kwds = {} - if parameter_type in ["integer", "float"]: + + if optional: + default_value = parameter_def.get("default", self.default_default_value) + parameter_kwds["value"] = default_value + + if "value" not in parameter_kwds and parameter_type in ["integer", "float"]: parameter_kwds["value"] = str(0) # TODO: Use a dict-based description from YAML tool source - element = Element("param", name="input", label=self.label, type=parameter_type, optional=str(optional), **parameter_kwds) + element = Element("param", name="input", label=self.label, type=parameter_type, optional=optional, **parameter_kwds) input = parameter_class(None, element) return dict(input=input) @@ -708,18 +884,78 @@ def get_all_outputs(self, data_only=False): if data_only: return [] + parameter_def = self._parse_state_into_dict() return [dict( name='output', label=self.label, - type=self.state.inputs.get('parameter_type', self.parameter_type), + type=parameter_def.get('parameter_type'), parameter=True, )] def execute(self, trans, progress, invocation_step, use_cached_job=False): step = invocation_step.workflow_step - step_outputs = dict(output=step.state.inputs['input']) + input_value = step.state.inputs['input'] + if input_value is None: + default_value = safe_loads(step.tool_inputs.get("default", "{}")) + # TODO: look at parameter type and infer if value should be a dictionary + # instead. Guessing only field parameter types in CWL branch would have + # default as dictionary like this. + if not isinstance(default_value, dict): + default_value = {"value": default_value} + input_value = default_value.get("value", NO_REPLACEMENT) + step_outputs = dict(output=input_value) progress.set_outputs_for_input(invocation_step, step_outputs) + def step_state_to_tool_state(self, state): + state = safe_loads(state) + default_set, default_value = False, None + if "default" in state: + default_set = True + default_value = state["default"] + state["optional"] = True + state = { + "parameter_definition": { + "parameter_type": state["parameter_type"], + "optional": { + "optional": str(state.get("optional", False)) + } + } + } + if default_set: + state["parameter_definition"]["optional"]["specify_default"] = {} + state["parameter_definition"]["optional"]["specify_default"]["specify_default"] = True + state["parameter_definition"]["optional"]["specify_default"]["default"] = default_value + state = json.dumps(state) + return state + + def get_export_state(self): + return self._parse_state_into_dict() + + def _parse_state_into_dict(self): + inputs = self.state.inputs + rval = {} + # 19.0X tool state... + if "parameter_type" in inputs: + rval.update({"parameter_type": inputs["parameter_type"], "optional": False}) + # expanded tool state... + elif "parameter_definition" in inputs: + parameters_def = inputs["parameter_definition"] + if "optional" in parameters_def: + optional_state = parameters_def["optional"] + optional = bool(optional_state["optional"]) + if "specify_default" in optional_state and bool(optional_state["specify_default"]["specify_default"]): + rval["default"] = optional_state["specify_default"]["default"] + else: + optional = False + rval.update({"parameter_type": parameters_def["parameter_type"], "optional": optional}) + else: + rval.update({"parameter_type": self.default_parameter_type, "optional": self.default_optional}) + return rval + + def save_to_step(self, step): + step.type = self.type + step.tool_inputs = self._parse_state_into_dict() + class PauseModule(WorkflowModule): """ Initially this module will unconditionally pause a workflow - will aim @@ -1236,7 +1472,7 @@ def callback(input, prefixed_name, **kwargs): try: # Replace DummyDatasets with historydatasetassociations - visit_input_values(tool.inputs, execution_state.inputs, callback, no_replacement_value=NO_REPLACEMENT) + visit_input_values(tool.inputs, execution_state.inputs, callback, no_replacement_value=NO_REPLACEMENT, replace_optional_connections=True) except KeyError as k: message_template = "Error due to input mapping of '%s' in '%s'. A common cause of this is conditional outputs that cannot be determined until runtime, please review your workflow." message = message_template % (tool.name, k.message) diff --git a/lib/galaxy/workflow/run.py b/lib/galaxy/workflow/run.py index 57bf4aaa3741..f3a6d5366f0b 100644 --- a/lib/galaxy/workflow/run.py +++ b/lib/galaxy/workflow/run.py @@ -414,9 +414,13 @@ def set_outputs_for_input(self, invocation_step, outputs=None, already_persisted if self.inputs_by_step_id: step_id = step.id if step_id not in self.inputs_by_step_id and 'output' not in outputs: - template = "Step with id %s not found in inputs_step_id (%s)" - message = template % (step_id, self.inputs_by_step_id) - raise ValueError(message) + default_value = step.input_default_value + if default_value: + outputs['output'] = default_value + else: + template = "Step with id %s not found in inputs_step_id (%s)" + message = template % (step.log_str(), self.inputs_by_step_id) + raise ValueError(message) elif step_id in self.inputs_by_step_id: outputs['output'] = self.inputs_by_step_id[step_id] diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index ff4950194388..2d08033c8462 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -82,10 +82,17 @@ def _normalize_inputs(steps, inputs, inputs_by): for possible_input_key in possible_input_keys: if possible_input_key in inputs: inputs_key = possible_input_key - if not inputs_key: - message = "Workflow cannot be run because an expected input step '%s' has no input dataset." % step.id + default_value = step.tool_inputs.get("default") + optional = step.tool_inputs.get("optional") or False + # Need to be careful here to make sure 'default' has correct type - not sure how to do that + # but asserting 'optional' is definitely a bool and not a String->Bool or something is a good + # start to ensure tool state is being preserved and loaded in a type safe way. + assert isinstance(optional, bool) + if not inputs_key and default_value is None and not optional: + message = "Workflow cannot be run because an expected input step '%s' (%s) is not optional and no input." % (step.id, step.label) raise exceptions.MessageException(message) - normalized_inputs[step.id] = inputs[inputs_key] + if inputs_key: + normalized_inputs[step.id] = inputs[inputs_key] return normalized_inputs diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 889d27d599c8..17a6deb9b893 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -21,6 +21,13 @@ WORKFLOW_NESTED_RUNTIME_PARAMETER, WORKFLOW_NESTED_SIMPLE, WORKFLOW_ONE_STEP_DEFAULT, + WORKFLOW_OPTIONAL_FALSE_INPUT_COLLECTION, + WORKFLOW_OPTIONAL_FALSE_INPUT_DATA, + WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, + WORKFLOW_OPTIONAL_TRUE_INPUT_DATA, + WORKFLOW_PARAMETER_INPUT_INTEGER_DEFAULT, + WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL, + WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED, WORKFLOW_RENAME_ON_INPUT, WORKFLOW_RUNTIME_PARAMETER_AFTER_PAUSE, WORKFLOW_WITH_CUSTOM_REPORT_1, @@ -493,8 +500,14 @@ def test_export(self): uploaded_workflow_id = self.workflow_populator.simple_workflow("test_for_export") downloaded_workflow = self._download_workflow(uploaded_workflow_id) assert downloaded_workflow["name"] == "test_for_export" - assert len(downloaded_workflow["steps"]) == 3 - first_input = downloaded_workflow["steps"]["0"]["inputs"][0] + steps = downloaded_workflow["steps"] + assert len(steps) == 3 + assert "0" in steps + first_step = steps["0"] + self._assert_has_keys(first_step, "inputs", "outputs") + inputs = first_step["inputs"] + assert len(inputs) > 0, first_step + first_input = inputs[0] assert first_input["name"] == "WorkflowInput1" assert first_input["description"] == "input1 description" self._assert_has_keys(downloaded_workflow, "a_galaxy_workflow", "format-version", "annotation", "uuid", "steps") @@ -2128,7 +2141,62 @@ def test_run_with_implicit_connection(self): self.wait_for_invocation_and_jobs(history_id, workflow_id, invocation_id) self._assert_history_job_count(history_id, 4) - def test_run_with_validated_parameter_connection_valid(self): + def test_run_with_optional_data_specified_to_multi_data(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_DATA, test_data=""" +input1: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "CCDS989.1_cds_0_0_chr1_147962193_r" in content + + def test_run_with_optional_data_unspecified_to_multi_data(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_DATA, test_data={}, history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "No input selected" in content + + def test_run_with_non_optional_data_unspecified_fails_invocation(self): + with self.dataset_populator.test_history() as history_id: + error = self._run_jobs(WORKFLOW_OPTIONAL_FALSE_INPUT_DATA, test_data={}, history_id=history_id, wait=False, assert_ok=False, expected_response=400) + self._assert_failed_on_non_optional_input(error, "input1") + + def test_run_with_optional_collection_specified(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, test_data=""" +input1: + type: paired + name: the_dataset_pair + elements: + - identifier: forward + value: 1.fastq + type: File + - identifier: reverse + value: 1.fastq + type: File +""", history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "GAATTGATCAGGACATAGGACAACTGTAGGCACCAT" in content + + def test_run_with_optional_collection_unspecified(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, test_data={}, history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "No input specified." in content + + def test_run_with_non_optional_collection_unspecified_fails_invocation(self): + with self.dataset_populator.test_history() as history_id: + error = self._run_jobs(WORKFLOW_OPTIONAL_FALSE_INPUT_COLLECTION, test_data={}, history_id=history_id, wait=False, assert_ok=False, expected_response=400) + self._assert_failed_on_non_optional_input(error, "input1") + + def _assert_failed_on_non_optional_input(self, error, input_name): + assert "err_msg" in error + err_msg = error["err_msg"] + assert input_name in err_msg + assert "is not optional and no input" in err_msg + + def test_run_with_validated_parameter_connection_optional(self): with self.dataset_populator.test_history() as history_id: run_summary = self._run_jobs(""" class: GalaxyWorkflow @@ -2150,6 +2218,48 @@ def test_run_with_validated_parameter_connection_valid(self): jobs = self._history_jobs(history_id) assert len(jobs) == 1 + def test_run_with_int_parameter(self): + with self.dataset_populator.test_history() as history_id: + failed = False + try: + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED, test_data=""" +data_input: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + except AssertionError as e: + assert '(int_input) is not optional' in str(e) + failed = True + assert failed + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED, test_data=""" +data_input: + value: 1.bed + type: File +int_input: + value: 1 + type: raw +""", history_id=history_id, wait=True, assert_ok=True) + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert len(content.splitlines()) == 1, content + + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL, test_data=""" +data_input: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + + def test_run_with_validated_parameter_connection_default_values(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_DEFAULT, test_data=""" +data_input: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert len(content.splitlines()) == 3, content + def test_run_with_validated_parameter_connection_invalid(self): with self.dataset_populator.test_history() as history_id: self._run_jobs(""" @@ -3415,7 +3525,7 @@ def test_workflow_import_state_validation_1(self): state: r2: - text: "" -""", history_id=history_id, wait=False, expected_response=400) +""", history_id=history_id, wait=False, expected_response=400, assert_ok=False) def _run_validation_workflow_with_substitions(self, substitions): workflow = self.workflow_populator.load_workflow_from_resource("test_workflow_validation_1") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 4a6815c915c1..1c1485845d6b 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -906,6 +906,9 @@ def run_workflow(self, has_workflow, test_data=None, history_id=None, wait=True, invocation_response = workflow_populator.invoke_workflow_raw(workflow_id, workflow_request) api_asserts.assert_status_code_is(invocation_response, expected_response) invocation = invocation_response.json() + if expected_response != 200: + assert not assert_ok + return invocation invocation_id = invocation.get('id') if invocation_id: # Wait for workflow to become fully scheduled and then for all jobs diff --git a/lib/galaxy_test/base/workflow_fixtures.py b/lib/galaxy_test/base/workflow_fixtures.py index 55de9ccc7e7d..33651b93ace6 100644 --- a/lib/galaxy_test/base/workflow_fixtures.py +++ b/lib/galaxy_test/base/workflow_fixtures.py @@ -316,6 +316,144 @@ """ +WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED = """ +class: GalaxyWorkflow +inputs: + data_input: data + int_input: integer +steps: + random: + tool_id: random_lines1 + in: + input: data_input + num_lines: int_input + state: + seed_source: + seed_source_selector: set_seed + seed: asdf +""" + + +WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL = """ +class: GalaxyWorkflow +inputs: + data_input: data + int_input: + type: integer + optional: true +steps: + random: + tool_id: random_lines1 + in: + input: data_input + num_lines: int_input + state: + seed_source: + seed_source_selector: set_seed + seed: asdf +""" + + +WORKFLOW_PARAMETER_INPUT_INTEGER_DEFAULT = """ +class: GalaxyWorkflow +inputs: + data_input: data + int_input: + type: integer + default: 3 +steps: + random: + tool_id: random_lines1 + in: + input: data_input + num_lines: int_input + state: + seed_source: + seed_source_selector: set_seed + seed: asdf +""" + + +WORKFLOW_OPTIONAL_TRUE_INPUT_DATA = """ +class: GalaxyWorkflow +inputs: + input1: + type: data + optional: true +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: multi_data_optional + in: + input1: input1 + out: + out1: out1 +""" + + +# Same workflow but non-optional input +WORKFLOW_OPTIONAL_FALSE_INPUT_DATA = """ +class: GalaxyWorkflow +inputs: + input1: + type: data + optional: false +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: multi_data_optional + in: + input1: input1 + out: + out1: out1 +""" + + +WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION = """ +class: GalaxyWorkflow +inputs: + input1: + type: collection + collection_type: paired + optional: true +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: collection_optional_param + in: + f1: input1 + out: + out1: out1 +""" + + +# Same workflow but non-optional input +WORKFLOW_OPTIONAL_FALSE_INPUT_COLLECTION = """ +class: GalaxyWorkflow +inputs: + input1: + type: collection + collection_type: paired + optional: false +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: collection_optional_param + in: + f1: input1 + out: + out1: out1 +""" + + WORKFLOW_RUNTIME_PARAMETER_SIMPLE = """ class: GalaxyWorkflow inputs: From 02440b62d957baab0877614bd0652d44d33d8eb4 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Fri, 6 Dec 2019 16:24:05 -0500 Subject: [PATCH 02/16] Skip recording output values if unlabeled (like data/collections). --- lib/galaxy/model/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index d9e61dbd973e..13a18ed8e8b9 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -5161,6 +5161,8 @@ def to_dict(self, view='collection', value_mapper=None, step_details=False, lega output_values = {} for output_param in self.output_values: label = output_param.workflow_output.label + if not label: + continue output_values[label] = output_param.value rval['output_values'] = output_values From 36549011442f41858601e23d992a18f1055ae6a9 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 9 Dec 2019 17:39:21 -0500 Subject: [PATCH 03/16] Restricted selection text inputs for workflow inputs. --- .../mvc/workflow/workflow-terminals.js | 7 ++- lib/galaxy/tool_util/parser/factory.py | 7 ++- lib/galaxy/workflow/modules.py | 45 +++++++++++++++---- test/functional/tools/simple_constructs.xml | 2 + 4 files changed, 49 insertions(+), 12 deletions(-) diff --git a/client/galaxy/scripts/mvc/workflow/workflow-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-terminals.js index fd82f3301519..c8ffc775ad05 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-terminals.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-terminals.js @@ -469,8 +469,13 @@ var InputParameterTerminal = BaseInputTerminal.extend({ connect: function(connector) { BaseInputTerminal.prototype.connect.call(this, connector); }, + effectiveType: function(parameterType) { + return parameterType == "select" ? "text" : parameterType; + }, attachable: function(other) { - return new ConnectionAcceptable(this.type == other.attributes.type, ""); + const effectiveThisType = this.effectiveType(this.type); + const effectiveOtherType = this.effectiveType(other.attributes.type); + return new ConnectionAcceptable(effectiveThisType == effectiveOtherType, ""); } }); diff --git a/lib/galaxy/tool_util/parser/factory.py b/lib/galaxy/tool_util/parser/factory.py index c17fc1165152..1029e396cfd2 100644 --- a/lib/galaxy/tool_util/parser/factory.py +++ b/lib/galaxy/tool_util/parser/factory.py @@ -10,7 +10,7 @@ from .cwl import CwlToolSource from .interface import InputSource from .xml import XmlInputSource, XmlToolSource -from .yaml import YamlToolSource +from .yaml import YamlInputSource, YamlToolSource from ..fetcher import ToolLocationFetcher log = logging.getLogger(__name__) @@ -82,7 +82,10 @@ def get_input_source(content): consume using the tool input source interface. """ if not isinstance(content, InputSource): - content = XmlInputSource(content) + if isinstance(content, dict): + content = YamlInputSource(content) + else: + content = XmlInputSource(content) return content diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 83fd3c20589b..ce756050df0b 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -51,6 +51,7 @@ from galaxy.tools.parameters.grouping import ( Conditional, ConditionalWhen, + Repeat, ) from galaxy.tools.parameters.history_query import HistoryQuery from galaxy.tools.parameters.wrapped import make_dict_copy @@ -814,10 +815,10 @@ def get_inputs(self): optional_cond.name = "optional" optional_cond.test_param = optional_value - when_text = ConditionalWhen() - when_text.value = param_type - when_text.inputs = OrderedDict() - when_text.inputs["optional"] = optional_cond + when_this_type = ConditionalWhen() + when_this_type.value = param_type + when_this_type.inputs = OrderedDict() + when_this_type.inputs["optional"] = optional_cond specify_default_checked = "default" in parameter_def specify_default = BooleanToolParameter(None, Element("param", name="specify_default", label="Specify a default value", type="boolean", checked=specify_default_checked)) @@ -849,7 +850,21 @@ def get_inputs(self): optional_cases = [when_true, when_false] optional_cond.cases = optional_cases - cases.append(when_text) + if param_type == "text": + # Repeats don't work - so use common separated list for now. + restrictions_list = parameter_def.get("restrictions") + if restrictions_list is None: + restrictions_list = [] + restriction_values = ",".join(restrictions_list) + restrictions = TextToolParameter(None, XML( + ''' + + + ''' % restriction_values)) + + when_this_type.inputs["restrictions"] = restrictions + + cases.append(when_this_type) parameter_type_cond.cases = cases return OrderedDict([("parameter_definition", parameter_type_cond)]) @@ -860,6 +875,9 @@ def get_runtime_inputs(self, **kwds): optional = parameter_def["optional"] if parameter_type not in ["text", "boolean", "integer", "float", "color"]: raise ValueError("Invalid parameter type for workflow parameters encountered.") + restrictions = parameter_type == "text" and parameter_def.get("restrictions") + if restrictions: + parameter_type = "select" parameter_class = parameter_types[parameter_type] parameter_kwds = {} @@ -870,9 +888,11 @@ def get_runtime_inputs(self, **kwds): if "value" not in parameter_kwds and parameter_type in ["integer", "float"]: parameter_kwds["value"] = str(0) - # TODO: Use a dict-based description from YAML tool source - element = Element("param", name="input", label=self.label, type=parameter_type, optional=optional, **parameter_kwds) - input = parameter_class(None, element) + if restrictions: + parameter_kwds["options"] = [{"value": r} for r in restrictions] + + input_source = dict(name="input", label=self.label, type=parameter_type, optional=optional, **parameter_kwds) + input = parameter_class(None, input_source) return dict(input=input) def get_runtime_state(self): @@ -913,6 +933,7 @@ def step_state_to_tool_state(self, state): default_set = True default_value = state["default"] state["optional"] = True + restrictions = state.get("restrictions") state = { "parameter_definition": { "parameter_type": state["parameter_type"], @@ -921,6 +942,8 @@ def step_state_to_tool_state(self, state): } } } + if restrictions: + state["parameter_definition"]["restrictions"] = ",".join(restrictions) if default_set: state["parameter_definition"]["optional"]["specify_default"] = {} state["parameter_definition"]["optional"]["specify_default"]["specify_default"] = True @@ -929,7 +952,8 @@ def step_state_to_tool_state(self, state): return state def get_export_state(self): - return self._parse_state_into_dict() + export_state = self._parse_state_into_dict() + return export_state def _parse_state_into_dict(self): inputs = self.state.inputs @@ -947,6 +971,9 @@ def _parse_state_into_dict(self): rval["default"] = optional_state["specify_default"]["default"] else: optional = False + restriction_values = parameters_def.get("restrictions") + if restriction_values: + rval.update({"restrictions": [v.strip() for v in restriction_values.split(",")]}) rval.update({"parameter_type": parameters_def["parameter_type"], "optional": optional}) else: rval.update({"parameter_type": self.default_parameter_type, "optional": self.default_optional}) diff --git a/test/functional/tools/simple_constructs.xml b/test/functional/tools/simple_constructs.xml index 6fdde0ff1540..eabf6e949cdd 100644 --- a/test/functional/tools/simple_constructs.xml +++ b/test/functional/tools/simple_constructs.xml @@ -7,7 +7,9 @@ echo "$radio_select" >> $out_file1; echo "$check_select" >> $out_file1; echo "$drop_select" >> $out_file1; + #if len($files) > 0 cat "$files[0].file" >> $out_file1; + #end if From 281961976a03689c824325d00891998dab19003b Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 9 Dec 2019 23:20:54 -0500 Subject: [PATCH 04/16] Allow specifying format on data/collection workflow inputs. --- lib/galaxy/workflow/modules.py | 59 +++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index ce756050df0b..674212140ce9 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -55,7 +55,7 @@ ) from galaxy.tools.parameters.history_query import HistoryQuery from galaxy.tools.parameters.wrapped import make_dict_copy -from galaxy.util import unicodify +from galaxy.util import listify, unicodify from galaxy.util.bunch import Bunch from galaxy.util.json import safe_loads from galaxy.util.rules_dsl import RuleSet @@ -587,6 +587,14 @@ def optional_param(optional): return optional_value +def format_param(trans, formats): + formats_val = "" if not formats else ",".join(formats) + source = dict(type="text", label="Format(s)", name="format", value=formats_val, optional=True, options=formats, help="Leave empty to auto-generate filtered list at runtime based on connections.") + source["options"] = [{"value": v, "label": v} for v in trans.app.datatypes_registry.datatypes_by_extension.keys()] + format_value = TextToolParameter(None, source) + return format_value + + class InputModule(WorkflowModule): default_optional = False @@ -641,9 +649,22 @@ def _parse_state_into_dict(self): else: optional = self.default_optional rval["optional"] = optional + if "format" in inputs: + formats = listify(inputs["format"]) + else: + formats = None + if formats: + rval["format"] = formats return rval def step_state_to_tool_state(self, state): + state = safe_loads(state) + if "format" in state: + formats = state["format"] + if formats: + formats = ",".join(formats) + state["format"] = formats + state = json.dumps(state) return state def save_to_step(self, step): @@ -672,9 +693,13 @@ def get_filter_set(self, connections=None): def get_runtime_inputs(self, connections=None): parameter_def = self._parse_state_into_dict() optional = parameter_def["optional"] - # TODO: extension from parameter_def - input_format = self.get_filter_set(connections) - input_param = DataToolParameter(None, Element("param", name="input", label=self.label, multiple=False, type="data", format=input_format, optional=str(optional)), self.trans) + formats = parameter_def.get("format") + if not formats: + formats = self.get_filter_set(connections) + else: + formats = ",".join(listify(formats)) + data_src = dict(name="input", label=self.label, multiple=False, type="data", format=formats, optional=optional) + input_param = DataToolParameter(None, data_src, self.trans) return dict(input=input_param) def get_inputs(self): @@ -682,6 +707,7 @@ def get_inputs(self): optional = parameter_def["optional"] inputs = OrderedDict() inputs["optional"] = optional_param(optional) + inputs["format"] = format_param(self.trans, parameter_def.get("format")) return inputs @@ -695,27 +721,28 @@ def get_inputs(self): parameter_def = self._parse_state_into_dict() collection_type = parameter_def["collection_type"] optional = parameter_def["optional"] - # TODO: extensions... - input_collection_type = TextToolParameter(None, XML( - ''' - - - - - - ''' % collection_type)) - + collection_type_source = dict(name="collection_type", label="Collection type", type="text", value=collection_type) + collection_type_source["options"] = [ + {"value": "list", "label": "List of Datasets"}, + {"value": "paired", "label": "Dataset Pair"}, + {"value": "list:paired", "label": "List of Dataset Pairs"}, + ] + input_collection_type = TextToolParameter(None, collection_type_source) inputs = OrderedDict() inputs["collection_type"] = input_collection_type inputs["optional"] = optional_param(optional) + inputs["format"] = format_param(self.trans, parameter_def.get("format")) return inputs def get_runtime_inputs(self, **kwds): parameter_def = self._parse_state_into_dict() collection_type = parameter_def["collection_type"] optional = parameter_def["optional"] - input_element = Element("param", name="input", label=self.label, type="data_collection", collection_type=collection_type, optional=str(optional)) - input_param = DataCollectionToolParameter(None, input_element, self.trans) + formats = parameter_def.get("format") + collection_param_source = dict(name="input", label=self.label, type="data_collection", collection_type=collection_type, optional=optional) + if formats: + collection_param_source["format"] = ",".join(listify(formats)) + input_param = DataCollectionToolParameter(None, collection_param_source, self.trans) return dict(input=input_param) def get_all_outputs(self, data_only=False): From 36d7b07d5c93fb47748e6027c132249f157b9e84 Mon Sep 17 00:00:00 2001 From: jraysajulga Date: Fri, 5 Jul 2019 05:24:33 -0500 Subject: [PATCH 05/16] Add select list option to workflow editor. Rebased of dozens of commits together by jraysajulga, mvdbeek, and jjohnson. Co-Authored-By: Jim Johnson Co-Authored-By: Marius van den Beek Co-Authored-By: John Chilton --- lib/galaxy/tool_util/parser/xml.py | 2 +- lib/galaxy/workflow/modules.py | 179 +++++++++++++++++++++++------ 2 files changed, 148 insertions(+), 33 deletions(-) diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index b453f1c82cab..9aaddfa09eec 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -1082,7 +1082,7 @@ def parse_static_options(self): return static_options def parse_optional(self, default=None): - """ Return boolean indicating wheter parameter is optional. """ + """ Return boolean indicating whether parameter is optional. """ elem = self.input_elem if self.get('type') == "data_column": # Allow specifing force_select for backward compat., but probably diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 674212140ce9..7922ec3cd148 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -4,7 +4,7 @@ import json import logging import re -from collections import OrderedDict +from collections import defaultdict, OrderedDict from xml.etree.ElementTree import ( Element, XML @@ -51,7 +51,6 @@ from galaxy.tools.parameters.grouping import ( Conditional, ConditionalWhen, - Repeat, ) from galaxy.tools.parameters.history_query import HistoryQuery from galaxy.tools.parameters.wrapped import make_dict_copy @@ -780,16 +779,16 @@ def get_inputs(self): parameter_def = self._parse_state_into_dict() parameter_type = parameter_def["parameter_type"] optional = parameter_def["optional"] - input_parameter_type = SelectToolParameter(None, XML( - ''' - - - - - - - - ''' % parameter_type)) + select_source = dict(name="parameter_type", label="Parameter type", type="select", value=parameter_type) + select_source["options"] = [ + {"value": "text", "label": "Text"}, + {"value": "integer", "label": "Integer"}, + {"value": "float", "label": "Float"}, + {"value": "boolean", "label": "Boolean (True or False)"}, + {"value": "color", "label": "Color"}, + ] + input_parameter_type = SelectToolParameter(None, select_source) + # encode following loop in description above instead for i, option in enumerate(input_parameter_type.static_options): option = list(option) if option[1] == parameter_type: @@ -878,36 +877,123 @@ def get_inputs(self): optional_cond.cases = optional_cases if param_type == "text": + restrict_how_source = dict(name="how", label="Restrict Text Values?", type="select") + if parameter_def.get("restrictions") is not None: + restrict_how_value = "staticRestrictions" + elif parameter_def.get("restrictOnConnections") is True: + restrict_how_value = "onConnections" + elif parameter_def.get("suggestions") is not None: + restrict_how_value = "staticSuggestions" + else: + restrict_how_value = "none" + log.info("parameter_def [%s], how [%s]" % (parameter_def, restrict_how_value)) + restrict_how_source["options"] = [ + {"value": "none", "label": "Do not specify restrictions (default).", "selected": restrict_how_value == "none"}, + {"value": "onConnections", "label": "Attempt restriction based on connections.", "selected": restrict_how_value == "onConnections"}, + {"value": "staticRestrictions", "label": "Provide list of restricted values.", "selected": restrict_how_value == "staticRestrictions"}, + {"value": "staticSuggestions", "label": "Provide list of suggested values.", "selected": restrict_how_value == "staticSuggestions"}, + ] + restrictions_cond = Conditional() + restrictions_how = SelectToolParameter(None, restrict_how_source) + restrictions_cond.name = "restrictions" + restrictions_cond.test_param = restrictions_how + + when_restrict_none = ConditionalWhen() + when_restrict_none.value = "none" + when_restrict_none.inputs = OrderedDict() + + when_restrict_connections = ConditionalWhen() + when_restrict_connections.value = "onConnections" + when_restrict_connections.inputs = OrderedDict() + + when_restrict_static_restrictions = ConditionalWhen() + when_restrict_static_restrictions.value = "staticRestrictions" + when_restrict_static_restrictions.inputs = OrderedDict() + + when_restrict_static_suggestions = ConditionalWhen() + when_restrict_static_suggestions.value = "staticSuggestions" + when_restrict_static_suggestions.inputs = OrderedDict() + # Repeats don't work - so use common separated list for now. - restrictions_list = parameter_def.get("restrictions") + + # Use both restrictions and suggestions as each other's default so value preserved. + restrictions_list = parameter_def.get("restrictions") or parameter_def.get("suggestions") if restrictions_list is None: restrictions_list = [] restriction_values = ",".join(restrictions_list) - restrictions = TextToolParameter(None, XML( - ''' - - - ''' % restriction_values)) + restrictions_source = dict(name="restrictions", label="Restriction Values", value=restriction_values, help="Comman-separated list of potential all values") + restrictions = TextToolParameter(None, restrictions_source) + + suggestions_source = dict(name="suggestions", label="Suggestion Values", value=restriction_values, help="Comman-separated list of some potential values") + suggestions = TextToolParameter(None, suggestions_source) + + when_restrict_static_restrictions.inputs["restrictions"] = restrictions + when_restrict_static_suggestions.inputs["suggestions"] = suggestions - when_this_type.inputs["restrictions"] = restrictions + restrictions_cond_cases = [when_restrict_none, when_restrict_connections, when_restrict_static_restrictions, when_restrict_static_suggestions] + restrictions_cond.cases = restrictions_cond_cases + when_this_type.inputs["restrictions"] = restrictions_cond cases.append(when_this_type) parameter_type_cond.cases = cases return OrderedDict([("parameter_definition", parameter_type_cond)]) - def get_runtime_inputs(self, **kwds): + def get_runtime_inputs(self, connections=None, **kwds): parameter_def = self._parse_state_into_dict() parameter_type = parameter_def["parameter_type"] optional = parameter_def["optional"] if parameter_type not in ["text", "boolean", "integer", "float", "color"]: raise ValueError("Invalid parameter type for workflow parameters encountered.") - restrictions = parameter_type == "text" and parameter_def.get("restrictions") - if restrictions: - parameter_type = "select" - parameter_class = parameter_types[parameter_type] + + # Optional parameters for tool input source definition. parameter_kwds = {} + is_text = parameter_type == "text" + restricted_inputs = False + + # Really is just an attempt - tool module may not be available (small problem), get_options may really depend on other + # values we are not setting, so this isn't great. Be sure to just fallback to text in this case. + attemptRestrictOnConnections = is_text and parameter_def.get("restrictOnConnections") and connections + try: + if attemptRestrictOnConnections: + static_options = [] + # Retrieve possible runtime options for 'select' type inputs + for connection in connections: + # Well this isn't a great assumption... + module = connection.input_step.module + tool_inputs = module.tool.inputs # may not be set, but we're catching the Exception below. + + def callback(input, prefixed_name, context, **kwargs): + if prefixed_name == connection.input_name: + static_options.append(input.get_options(self.trans, {})) + visit_input_values(tool_inputs, module.state.inputs, callback) + + if static_options: + # Intersection based on values. + intxn_vals = set.intersection(*({option[1] for option in options} for options in static_options)) + intxn_opts = {option for options in static_options for option in options if option[1] in intxn_vals} + d = defaultdict(set) # Collapse labels with same values + for label, value, selected in intxn_opts: + d[value].add(label) + options = [{"label": ', '.join(label), "value": value, "selected": False} for value, label in d.items()] + if options: + parameter_kwds["options"] = options + restricted_inputs = True + except Exception: + log.debug("Failed to generate options for text parameter, falling back to free text.", exc_info=True) + + if is_text and not restricted_inputs and parameter_def.get("restrictions"): + restriction_values = parameter_def.get("restrictions") + parameter_kwds["options"] = [{"value": r} for r in restriction_values] + restricted_inputs = True + + client_parameter_type = parameter_type + if restricted_inputs: + client_parameter_type = "select" + + parameter_class = parameter_types[client_parameter_type] + if optional: default_value = parameter_def.get("default", self.default_default_value) parameter_kwds["value"] = default_value @@ -915,10 +1001,11 @@ def get_runtime_inputs(self, **kwds): if "value" not in parameter_kwds and parameter_type in ["integer", "float"]: parameter_kwds["value"] = str(0) - if restrictions: - parameter_kwds["options"] = [{"value": r} for r in restrictions] + if is_text and parameter_def.get("suggestions") is not None: + suggestion_values = parameter_def.get("suggestions") + parameter_kwds["options"] = [{"value": r} for r in suggestion_values] - input_source = dict(name="input", label=self.label, type=parameter_type, optional=optional, **parameter_kwds) + input_source = dict(name="input", label=self.label, type=client_parameter_type, optional=optional, **parameter_kwds) input = parameter_class(None, input_source) return dict(input=input) @@ -961,6 +1048,16 @@ def step_state_to_tool_state(self, state): default_value = state["default"] state["optional"] = True restrictions = state.get("restrictions") + restrictOnConnections = state.get("restrictOnConnections") + suggestions = state.get("suggestions") + restrictions_how = "none" + if restrictions is not None: + restrictions_how = "staticRestrictions" + if suggestions is not None: + restrictions_how = "staticSuggestions" + elif restrictOnConnections: + restrictions_how = "onConnections" + state = { "parameter_definition": { "parameter_type": state["parameter_type"], @@ -969,8 +1066,12 @@ def step_state_to_tool_state(self, state): } } } - if restrictions: - state["parameter_definition"]["restrictions"] = ",".join(restrictions) + state["parameter_definition"]["restrictions"] = {} + state["parameter_definition"]["restrictions"]["how"] = restrictions_how + if restrictions_how == "staticRestrictions": + state["parameter_definition"]["restrictions"]["restrictions"] = ",".join(restrictions) + if restrictions_how == "staticSuggestions": + state["parameter_definition"]["restrictions"]["suggestions"] = ",".join(suggestions) if default_set: state["parameter_definition"]["optional"]["specify_default"] = {} state["parameter_definition"]["optional"]["specify_default"]["specify_default"] = True @@ -998,9 +1099,23 @@ def _parse_state_into_dict(self): rval["default"] = optional_state["specify_default"]["default"] else: optional = False - restriction_values = parameters_def.get("restrictions") - if restriction_values: - rval.update({"restrictions": [v.strip() for v in restriction_values.split(",")]}) + restrictions_cond_values = parameters_def.get("restrictions") + if restrictions_cond_values: + # how better be in here. + how = restrictions_cond_values["how"] + if how == "none": + pass + elif how == "onConnections": + rval["restrictOnConnections"] = True + elif how == "staticRestrictions": + restriction_values = restrictions_cond_values["restrictions"] + rval.update({"restrictions": [v.strip() for v in restriction_values.split(",")]}) + elif how == "staticSuggestions": + suggestion_values = restrictions_cond_values["suggestions"] + rval.update({"suggestions": [v.strip() for v in suggestion_values.split(",")]}) + else: + log.warn("Unknown restriction conditional type encountered for workflow parameter.") + rval.update({"parameter_type": parameters_def["parameter_type"], "optional": optional}) else: rval.update({"parameter_type": self.default_parameter_type, "optional": self.default_optional}) From b9cf792d0d80f7bb7729e9ee224fd63145a3a47d Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 10 Dec 2019 13:29:02 -0500 Subject: [PATCH 06/16] Optimize user experience of connecting to one option. --- lib/galaxy/workflow/modules.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 7922ec3cd148..134c80949184 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -969,17 +969,22 @@ def callback(input, prefixed_name, context, **kwargs): static_options.append(input.get_options(self.trans, {})) visit_input_values(tool_inputs, module.state.inputs, callback) - if static_options: - # Intersection based on values. + options = None + if static_options and len(static_options) == 1: + # If we are connected to a single option, just use it as is so order is preserved cleanly and such. + options = [{"value": o[0], "label": o[1]} for o in static_options[0]] + elif static_options: + # Intersection based on values of multiple option connections. intxn_vals = set.intersection(*({option[1] for option in options} for options in static_options)) intxn_opts = {option for options in static_options for option in options if option[1] in intxn_vals} d = defaultdict(set) # Collapse labels with same values for label, value, selected in intxn_opts: d[value].add(label) options = [{"label": ', '.join(label), "value": value, "selected": False} for value, label in d.items()] - if options: - parameter_kwds["options"] = options - restricted_inputs = True + + if options is not None: + parameter_kwds["options"] = options + restricted_inputs = True except Exception: log.debug("Failed to generate options for text parameter, falling back to free text.", exc_info=True) From de1e11cd82edda299af85d61edc5e24af7fa072a Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 10 Dec 2019 14:08:44 -0500 Subject: [PATCH 07/16] Allow labels on restriction/suggestion values. --- lib/galaxy/tools/parameters/basic.py | 2 ++ lib/galaxy/workflow/modules.py | 47 +++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 3dfcbcb66b5f..27ad9586f84b 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -897,6 +897,8 @@ def from_json(self, value, trans, other_values={}, require_legal_value=True): return [] else: raise ValueError("parameter '%s': no option was selected for non optional parameter" % (self.name)) + if is_runtime_value(value): + return None if value not in legal_values and require_legal_value: raise ValueError("parameter '%s': an invalid option (%r) was selected (valid options: %s)" % (self.name, value, ",".join(legal_values))) return value diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 134c80949184..a09fa14ae912 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -920,7 +920,7 @@ def get_inputs(self): restrictions_list = parameter_def.get("restrictions") or parameter_def.get("suggestions") if restrictions_list is None: restrictions_list = [] - restriction_values = ",".join(restrictions_list) + restriction_values = self._parameter_option_def_to_tool_form_str(restrictions_list) restrictions_source = dict(name="restrictions", label="Restriction Values", value=restriction_values, help="Comman-separated list of potential all values") restrictions = TextToolParameter(None, restrictions_source) @@ -988,9 +988,26 @@ def callback(input, prefixed_name, context, **kwargs): except Exception: log.debug("Failed to generate options for text parameter, falling back to free text.", exc_info=True) + def _parameter_def_list_to_options(parameter_value): + options = [] + for item in parameter_value: + option = {} + if isinstance(item, dict): + value = item["value"] + option["value"] = value + if "label" in item: + option["label"] = item["label"] + else: + option["label"] = value + else: + option["value"] = item + option["label"] = item + options.append(option) + return options + if is_text and not restricted_inputs and parameter_def.get("restrictions"): restriction_values = parameter_def.get("restrictions") - parameter_kwds["options"] = [{"value": r} for r in restriction_values] + parameter_kwds["options"] = _parameter_def_list_to_options(restriction_values) restricted_inputs = True client_parameter_type = parameter_type @@ -1008,7 +1025,7 @@ def callback(input, prefixed_name, context, **kwargs): if is_text and parameter_def.get("suggestions") is not None: suggestion_values = parameter_def.get("suggestions") - parameter_kwds["options"] = [{"value": r} for r in suggestion_values] + parameter_kwds["options"] = _parameter_def_list_to_options(suggestion_values) input_source = dict(name="input", label=self.label, type=client_parameter_type, optional=optional, **parameter_kwds) input = parameter_class(None, input_source) @@ -1073,10 +1090,11 @@ def step_state_to_tool_state(self, state): } state["parameter_definition"]["restrictions"] = {} state["parameter_definition"]["restrictions"]["how"] = restrictions_how + if restrictions_how == "staticRestrictions": - state["parameter_definition"]["restrictions"]["restrictions"] = ",".join(restrictions) + state["parameter_definition"]["restrictions"]["restrictions"] = self._parameter_option_def_to_tool_form_str(restrictions) if restrictions_how == "staticSuggestions": - state["parameter_definition"]["restrictions"]["suggestions"] = ",".join(suggestions) + state["parameter_definition"]["restrictions"]["suggestions"] = self._parameter_option_def_to_tool_form_str(suggestions) if default_set: state["parameter_definition"]["optional"]["specify_default"] = {} state["parameter_definition"]["optional"]["specify_default"]["specify_default"] = True @@ -1084,6 +1102,9 @@ def step_state_to_tool_state(self, state): state = json.dumps(state) return state + def _parameter_option_def_to_tool_form_str(self, parameter_def): + return ",".join("%s:%s" % (o["value"], o["label"]) if isinstance(o, dict) else o for o in parameter_def) + def get_export_state(self): export_state = self._parse_state_into_dict() return export_state @@ -1106,6 +1127,18 @@ def _parse_state_into_dict(self): optional = False restrictions_cond_values = parameters_def.get("restrictions") if restrictions_cond_values: + + def _string_to_parameter_def_option(str_value): + items = [] + for option_part in str_value.split(","): + option_part = option_part.strip() + if ":" not in option_part: + items.append(option_part) + else: + value, label = option_part.split(":", 1) + items.append({"value": value, "label": label}) + return items + # how better be in here. how = restrictions_cond_values["how"] if how == "none": @@ -1114,10 +1147,10 @@ def _parse_state_into_dict(self): rval["restrictOnConnections"] = True elif how == "staticRestrictions": restriction_values = restrictions_cond_values["restrictions"] - rval.update({"restrictions": [v.strip() for v in restriction_values.split(",")]}) + rval.update({"restrictions": _string_to_parameter_def_option(restriction_values)}) elif how == "staticSuggestions": suggestion_values = restrictions_cond_values["suggestions"] - rval.update({"suggestions": [v.strip() for v in suggestion_values.split(",")]}) + rval.update({"suggestions": _string_to_parameter_def_option(suggestion_values)}) else: log.warn("Unknown restriction conditional type encountered for workflow parameter.") From 36bc4e3b379ca66c05f9703bd14c1adbf4512d76 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 10 Dec 2019 14:14:12 -0500 Subject: [PATCH 08/16] Fix language a bit in restriction type selection box. --- lib/galaxy/workflow/modules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index a09fa14ae912..db2a2b4f5c20 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -890,7 +890,7 @@ def get_inputs(self): restrict_how_source["options"] = [ {"value": "none", "label": "Do not specify restrictions (default).", "selected": restrict_how_value == "none"}, {"value": "onConnections", "label": "Attempt restriction based on connections.", "selected": restrict_how_value == "onConnections"}, - {"value": "staticRestrictions", "label": "Provide list of restricted values.", "selected": restrict_how_value == "staticRestrictions"}, + {"value": "staticRestrictions", "label": "Provide list of all possible values.", "selected": restrict_how_value == "staticRestrictions"}, {"value": "staticSuggestions", "label": "Provide list of suggested values.", "selected": restrict_how_value == "staticSuggestions"}, ] restrictions_cond = Conditional() From 20d737cd0906798624379986e40d403dd56b645c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 10 Dec 2019 21:21:31 -0500 Subject: [PATCH 09/16] Fix unit tests. --- lib/galaxy/workflow/modules.py | 4 +++- test/unit/workflows/workflow_support.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index db2a2b4f5c20..5ac118415116 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -165,7 +165,9 @@ def recover_state(self, state, **kwds): """ from_tool_form = kwds.get("from_tool_form", False) if not from_tool_form: - state = self.step_state_to_tool_state(state) + # I've never seen state here be none except for unit tests so 'or {}' below may only be + # needed due to test bugs. Doesn't hurt anything though. + state = self.step_state_to_tool_state(state or {}) self.state = DefaultToolState() inputs = self.get_inputs() diff --git a/test/unit/workflows/workflow_support.py b/test/unit/workflows/workflow_support.py index 534b71bb29f8..234ecbd32c79 100644 --- a/test/unit/workflows/workflow_support.py +++ b/test/unit/workflows/workflow_support.py @@ -58,6 +58,10 @@ def __init__(self): def get_datatype_by_extension(self, ext): return ext + @property + def datatypes_by_extension(self): + return {"fasta": object(), "fastqsanger": object(), "txt": object()} + class TestToolbox(object): From 1fd3b9fbcfe019b978648892f741e6745b54da49 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 08:02:53 -0500 Subject: [PATCH 10/16] Update docstring for get_input_source. --- lib/galaxy/tool_util/parser/factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/parser/factory.py b/lib/galaxy/tool_util/parser/factory.py index 1029e396cfd2..41c9ba3001e3 100644 --- a/lib/galaxy/tool_util/parser/factory.py +++ b/lib/galaxy/tool_util/parser/factory.py @@ -75,7 +75,7 @@ def get_tool_source_from_representation(tool_format, tool_representation): def get_input_source(content): - """Wrap an XML element in a XmlInputSource if needed. + """Wrap dicts or XML elements as InputSource if needed. If the supplied content is already an InputSource object, it is simply returned. This allow Galaxy to uniformly From 4ec4f3c3880f292620db1239e17cc98898193386 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 08:09:21 -0500 Subject: [PATCH 11/16] Dictified representation instead of XML for more module param decls. --- lib/galaxy/workflow/modules.py | 45 ++++++++++++---------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 5ac118415116..4c9faf91bdaa 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -5,10 +5,6 @@ import logging import re from collections import defaultdict, OrderedDict -from xml.etree.ElementTree import ( - Element, - XML -) from galaxy import ( exceptions, @@ -584,7 +580,8 @@ def to_dict(self, *args, **kwds): def optional_param(optional): - optional_value = BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", checked=optional)) + bool_source = dict(name="optional", label="Optional", type="boolean", checked=optional) + optional_value = BooleanToolParameter(None, bool_source) return optional_value @@ -804,41 +801,30 @@ def get_inputs(self): cases = [] for param_type in ["text", "integer", "float"]: + default_source = dict(name="default", label="Default Value", type=parameter_type) if param_type == "text": if parameter_type == "text": default = parameter_def.get("default") or "" else: default = "" - input_default_value = TextToolParameter(None, XML( - ''' - - - ''' - % default)) + default_source["value"] = default + input_default_value = TextToolParameter(None, default_source) elif param_type == "integer": if parameter_type == "integer": - default = str(parameter_def.get("default") or "0") + default = parameter_def.get("default") or 0 else: - default = "0" - input_default_value = IntegerToolParameter(None, XML( - ''' - - - ''' - % default)) + default = 0 + default_source["value"] = default + input_default_value = IntegerToolParameter(None, default_source) elif param_type == "float": if parameter_type == "float": - default = str(parameter_def.get("default") or "0.0") + default = parameter_def.get("default") or 0.0 else: - default = "0.0" - input_default_value = FloatToolParameter(None, XML( - ''' - - - ''' - % default)) + default = 0.0 + default_source["value"] = default + input_default_value = FloatToolParameter(None, default_source) # color parameter defaults? - optional_value = BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", checked=optional)) + optional_value = optional_param(optional) optional_cond = Conditional() optional_cond.name = "optional" optional_cond.test_param = optional_value @@ -849,7 +835,8 @@ def get_inputs(self): when_this_type.inputs["optional"] = optional_cond specify_default_checked = "default" in parameter_def - specify_default = BooleanToolParameter(None, Element("param", name="specify_default", label="Specify a default value", type="boolean", checked=specify_default_checked)) + specify_default_source = dict(name="specify_default", label="Specify a default value", type="boolean", checked=specify_default_checked) + specify_default = BooleanToolParameter(None, specify_default_source) specify_default_cond = Conditional() specify_default_cond.name = "specify_default" specify_default_cond.test_param = specify_default From 26d51502456a5ce9b34a418e5caf742dcff2bdda Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 08:23:40 -0500 Subject: [PATCH 12/16] Eliminate extensions=['input_collection']. The existing hack of extensions=['input'] is bad enough and should work fine for collections. Not sure what past-John was thinking by adding this. --- client/galaxy/scripts/mvc/workflow/workflow-terminals.js | 3 +-- client/galaxy/scripts/mvc/workflow/workflow-view-data.js | 2 +- lib/galaxy/workflow/modules.py | 2 +- test/unit/workflows/test_modules.py | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/client/galaxy/scripts/mvc/workflow/workflow-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-terminals.js index c8ffc775ad05..0a1731abf0f3 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-terminals.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-terminals.js @@ -312,7 +312,7 @@ var BaseInputTerminal = Terminal.extend({ if ( firstOutput.isCollection || firstOutput.isMappedOver() || - firstOutput.datatypes.indexOf("input_collection") > 0 + firstOutput.datatypes.indexOf("input") > 0 ) { return true; } else { @@ -359,7 +359,6 @@ var BaseInputTerminal = Terminal.extend({ if ( other_datatype == "input" || other_datatype == "_sniff_" || - other_datatype == "input_collection" || window.workflow_globals.app.isSubType(cat_outputs[other_datatype_i], thisDatatype) ) { return new ConnectionAcceptable(true, null); diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-data.js b/client/galaxy/scripts/mvc/workflow/workflow-view-data.js index 0d57a0979737..35b57b23901b 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-view-data.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-view-data.js @@ -47,7 +47,7 @@ const DataOutputView = Backbone.View.extend({ let label = output.label || output.name; const node = this.nodeView.node; - const isInput = output.extensions.indexOf("input") >= 0 || output.extensions.indexOf("input_collection") >= 0; + const isInput = output.extensions.indexOf("input") >= 0; if (!isInput) { label = `${label} (${output.force_datatype || output.extensions.join(", ")})`; } diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 4c9faf91bdaa..f8d3fd04793f 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -747,7 +747,7 @@ def get_all_outputs(self, data_only=False): return [ dict( name='output', - extensions=['input_collection'], + extensions=['input'], collection=True, collection_type=self.state.inputs.get('collection_type', self.default_collection_type) ) diff --git a/test/unit/workflows/test_modules.py b/test/unit/workflows/test_modules.py index 4ad43a127f2b..8327493a6d72 100644 --- a/test/unit/workflows/test_modules.py +++ b/test/unit/workflows/test_modules.py @@ -109,7 +109,7 @@ def test_data_collection_input_connections(): assert len(outputs) == 1 output = outputs[0] assert output["name"] == "output" - assert output["extensions"] == ["input_collection"] + assert output["extensions"] == ["input"] assert output["collection_type"] == "list:paired" From 7ba061e936b55447adcd5631d5a9a97ad6c25c46 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 08:24:51 -0500 Subject: [PATCH 13/16] Use annotated format information in workflow editor. --- lib/galaxy/workflow/modules.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index f8d3fd04793f..46085a3e00e8 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -675,7 +675,13 @@ class InputDataModule(InputModule): name = "Input dataset" def get_all_outputs(self, data_only=False): - return [dict(name='output', extensions=['input'])] + parameter_def = self._parse_state_into_dict() + format_def = parameter_def.get("format") + if format_def is None: + extensions = ['input'] + else: + extensions = listify(format_def) + return [dict(name='output', extensions=extensions)] def get_filter_set(self, connections=None): filter_set = [] From 44b63ff8b354855a969233c45e9e5cfa04cc748c Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 09:18:04 -0500 Subject: [PATCH 14/16] Update workflow editor for optional outputs. --- .../scripts/mvc/workflow/workflow-node.js | 1 + .../mvc/workflow/workflow-terminals.js | 32 +++++++++++++++---- .../mvc/workflow/workflow-view-terminals.js | 11 ++++--- .../qunit/tests/workflow_editor_tests.js | 29 +++++++++++++++-- lib/galaxy/workflow/modules.py | 20 ++++++++++-- 5 files changed, 77 insertions(+), 16 deletions(-) diff --git a/client/galaxy/scripts/mvc/workflow/workflow-node.js b/client/galaxy/scripts/mvc/workflow/workflow-node.js index a93b421df473..d2d4ddce06ec 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-node.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-node.js @@ -297,6 +297,7 @@ var Node = Backbone.Model.extend({ if (node.type == "parameter_input") { node.output_terminals[output.name].attributes.type = output.type; } + node.output_terminals[output.name].optional = output.optional; node.output_terminals[output.name].destroyInvalidConnections(); } }); diff --git a/client/galaxy/scripts/mvc/workflow/workflow-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-terminals.js index 0a1731abf0f3..9099fcd2c027 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-terminals.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-terminals.js @@ -212,9 +212,16 @@ var OutputTerminal = Terminal.extend({ initialize: function(attr) { Terminal.prototype.initialize.call(this, attr); this.datatypes = attr.datatypes; + this.optional = attr.optional; this.force_datatype = attr.force_datatype; }, - + /* + update: function(output) { + // This isn't needed for datatypes or optional because WorkflowNode.update_field_data just updates the + // properties directly, but an upshot of that is that connected non-optional inputs that become optional + // don't disconnect from non-optional inputs. + }, + */ resetMappingIfNeeded: function() { // If inputs were only mapped over to preserve // an output just disconnected reset these... @@ -370,6 +377,12 @@ var BaseInputTerminal = Terminal.extend({ `Effective output data type(s) [${cat_outputs}] do not appear to match input type(s) [${this.datatypes}].` ); }, + _producesAcceptableDatatypeAndOptionalness(other) { + if (!this.optional && !this.multiple && other.optional) { + return new ConnectionAcceptable(false, "Cannot connect an optional output to a non-optional input"); + } + return this._producesAcceptableDatatype(other); + }, _otherCollectionType: function(other) { var otherCollectionType = NULL_COLLECTION_TYPE_DESCRIPTION; if (other.isCollection) { @@ -387,6 +400,7 @@ var InputTerminal = BaseInputTerminal.extend({ update: function(input) { this.datatypes = input.extensions; this.multiple = input.multiple; + this.optional = input.optional; this.collection = false; }, connect: function(connector) { @@ -419,16 +433,16 @@ var InputTerminal = BaseInputTerminal.extend({ } } if (thisMapOver.isCollection && thisMapOver.canMatch(otherCollectionType)) { - return this._producesAcceptableDatatype(other); + return this._producesAcceptableDatatypeAndOptionalness(other); } else if (this.multiple && new CollectionTypeDescription("list").canMatch(otherCollectionType)) { // This handles the special case of a list input being connected to a multiple="true" data input. // Nested lists would be correctly mapped over by the above condition. - return this._producesAcceptableDatatype(other); + return this._producesAcceptableDatatypeAndOptionalness(other); } else { // Need to check if this would break constraints... var mappingConstraints = this._mappingConstraints(); if (mappingConstraints.every(_.bind(otherCollectionType.canMatch, otherCollectionType))) { - return this._producesAcceptableDatatype(other); + return this._producesAcceptableDatatypeAndOptionalness(other); } else { if (thisMapOver.isCollection) { // incompatible collection type attached @@ -457,13 +471,14 @@ var InputTerminal = BaseInputTerminal.extend({ "Cannot attach non-collection outputs to mapped over inputs, consider disconnecting inputs and outputs to reset this input's mapping." ); } - return this._producesAcceptableDatatype(other); + return this._producesAcceptableDatatypeAndOptionalness(other); } }); var InputParameterTerminal = BaseInputTerminal.extend({ update: function(input) { this.type = input.type; + this.optional = input.optional; }, connect: function(connector) { BaseInputTerminal.prototype.connect.call(this, connector); @@ -484,6 +499,7 @@ var InputCollectionTerminal = BaseInputTerminal.extend({ this.collection = true; this.collection_type = input.collection_type; this.datatypes = input.extensions; + this.optional = input.optional; var collectionTypes = []; if (input.collection_types) { _.each(input.collection_types, collectionType => { @@ -554,7 +570,7 @@ var InputCollectionTerminal = BaseInputTerminal.extend({ ); if (canMatch) { // Only way a direct match... - return this._producesAcceptableDatatype(other); + return this._producesAcceptableDatatypeAndOptionalness(other); // Otherwise we need to mapOver } else if (thisMapOver.isCollection) { // In this case, mapOver already set and we didn't match skipping... @@ -578,7 +594,7 @@ var InputCollectionTerminal = BaseInputTerminal.extend({ // Need to check if this would break constraints... var mappingConstraints = this._mappingConstraints(); if (mappingConstraints.every(d => effectiveMapOver.canMatch(d))) { - return this._producesAcceptableDatatype(other); + return this._producesAcceptableDatatypeAndOptionalness(other); } else { if (this.node.hasConnectedMappedInputTerminals()) { return new ConnectionAcceptable( @@ -605,6 +621,7 @@ var OutputCollectionTerminal = Terminal.extend({ initialize: function(attr) { Terminal.prototype.initialize.call(this, attr); this.datatypes = attr.datatypes; + this.optional = attr.optional; this.force_datatype = attr.force_datatype; if (attr.collection_type) { this.collectionType = new CollectionTypeDescription(attr.collection_type); @@ -618,6 +635,7 @@ var OutputCollectionTerminal = Terminal.extend({ this.isCollection = true; }, update: function(output) { + this.optional = output.optional; var newCollectionType; if (output.collection_type) { newCollectionType = new CollectionTypeDescription(output.collection_type); diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js index dd2b534c89a0..7614dfa71c1f 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-view-terminals.js @@ -411,7 +411,8 @@ var OutputTerminalView = BaseOutputTerminalView.extend({ return new Terminals.OutputTerminal({ element: this.el, datatypes: type, - force_datatype: output.force_datatype + force_datatype: output.force_datatype, + optional: output.optional }); } }); @@ -427,7 +428,8 @@ var OutputCollectionTerminalView = BaseOutputTerminalView.extend({ collection_type: collection_type, collection_type_source: collection_type_source, datatypes: output.extensions, - force_datatype: output.force_datatype + force_datatype: output.force_datatype, + optional: output.optional }); } }); @@ -436,9 +438,10 @@ var OutputParameterTerminalView = BaseOutputTerminalView.extend({ terminalMappingClass: Terminals.TerminalMapping, terminalMappingViewClass: TerminalMappingView, terminalForOutput: function(output) { - return new Terminals.OutputCollectionTerminal({ + return new Terminals.OutputParameterTerminal({ element: this.el, - type: output.type + type: output.type, + optional: output.optional }); } }); diff --git a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js index 77d2812c8a4a..a04c6c2ccce5 100644 --- a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js +++ b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js @@ -39,7 +39,7 @@ QUnit.module("Input terminal model test", { beforeEach: function() { testApp.create(); this.node = new Node(create_app(), {}); - this.input = { extensions: ["txt"], multiple: false }; + this.input = { extensions: ["txt"], multiple: false, optional: false }; this.input_terminal = new Terminals.InputTerminal({ input: this.input }); this.input_terminal.node = this.node; }, @@ -65,7 +65,7 @@ QUnit.module("Input terminal model test", { this.input_terminal.connectors = []; }, test_accept: function(other) { - other = other || { node: {}, datatypes: ["txt"] }; + other = other || { node: {}, datatypes: ["txt"], optional: false }; if (!other.mapOver) { other.mapOver = function() { return Terminals.NULL_COLLECTION_TYPE_DESCRIPTION; @@ -151,6 +151,20 @@ QUnit.test("can accept inputs", function(assert) { assert.ok(this.test_accept(other)); }); +QUnit.test("can't connect non-optional", function(assert) { + var other = { node: {}, datatypes: ["input"], optional: true }; + assert.ok(!this.test_accept(other)); +}); + +QUnit.test("multiple inputs can accept optional outputs regardless", function(assert) { + // Galaxy multiple inputs have an optional field but it is hard to resolve that + // completely until runtime. + var other = { node: {}, datatypes: ["input"], optional: true }; + var self = this; + this.multiple(); + assert.ok(this.test_accept(other)); +}); + QUnit.test("input type can accept any datatype", function(assert) { this.input.extensions = ["input"]; this.input_terminal.update(this.input); @@ -304,6 +318,17 @@ QUnit.test("Collection output can connect to same collection input type", functi ); }); +QUnit.test("Optiona collection output can not connect to required collection input", function(assert) { + var inputTerminal = this.input_terminal; + var outputTerminal = new Terminals.OutputCollectionTerminal({ + datatypes: "txt", + collection_type: "list", + optional: true + }); + outputTerminal.node = {}; + assert.ok(!inputTerminal.canAccept(outputTerminal).canAccept); +}); + QUnit.test("Collection output cannot connect to different collection input type", function(assert) { var self = this; var inputTerminal = self.input_terminal; diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 46085a3e00e8..31a2348c9265 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -677,11 +677,12 @@ class InputDataModule(InputModule): def get_all_outputs(self, data_only=False): parameter_def = self._parse_state_into_dict() format_def = parameter_def.get("format") + optional = parameter_def["optional"] if format_def is None: extensions = ['input'] else: extensions = listify(format_def) - return [dict(name='output', extensions=extensions)] + return [dict(name='output', extensions=extensions, optional=optional)] def get_filter_set(self, connections=None): filter_set = [] @@ -750,12 +751,20 @@ def get_runtime_inputs(self, **kwds): return dict(input=input_param) def get_all_outputs(self, data_only=False): + parameter_def = self._parse_state_into_dict() + format_def = parameter_def.get("format") + optional = parameter_def["optional"] + if format_def is None: + extensions = ['input'] + else: + extensions = listify(format_def) return [ dict( name='output', - extensions=['input'], + extensions=extensions, collection=True, - collection_type=self.state.inputs.get('collection_type', self.default_collection_type) + collection_type=parameter_def.get('collection_type', self.default_collection_type), + optional=optional, ) ] @@ -1040,6 +1049,7 @@ def get_all_outputs(self, data_only=False): name='output', label=self.label, type=parameter_def.get('parameter_type'), + optional=parameter_def['optional'], parameter=True, )] @@ -1373,6 +1383,7 @@ def callback(input, prefixed_name, prefixed_label, value=None, **kwargs): label=prefixed_label, multiple=input.multiple, extensions=input.extensions, + optional=input.optional, input_type="dataset", )) elif isinstance(input, DataCollectionToolParameter): inputs.append(dict( @@ -1381,6 +1392,7 @@ def callback(input, prefixed_name, prefixed_label, value=None, **kwargs): multiple=input.multiple, input_type="dataset_collection", collection_types=input.collection_types, + optional=input.optional, extensions=input.extensions, )) else: @@ -1390,6 +1402,7 @@ def callback(input, prefixed_name, prefixed_label, value=None, **kwargs): label=prefixed_label, multiple=False, input_type="parameter", + optional=getattr(input, "optional", False), type=input_type, ) ) @@ -1445,6 +1458,7 @@ def get_all_outputs(self, data_only=False): name=name, extensions=formats, type=tool_output.output_type, + optional=False, **extra_kwds ) ) From 68e6d8f68d3a013c72f7d18bf48c23f415239fc2 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 09:19:55 -0500 Subject: [PATCH 15/16] Eliminate some uses of self in workflow editor tests. --- .../qunit/tests/workflow_editor_tests.js | 33 +++++++------------ 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js index a04c6c2ccce5..225738042da0 100644 --- a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js +++ b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js @@ -173,35 +173,31 @@ QUnit.test("input type can accept any datatype", function(assert) { }); QUnit.test("cannot accept when already connected", function(assert) { - var self = this; // If other is subtype but already connected, cannot accept - this.with_test_connector(function() { - assert.ok(!self.test_accept()); + this.with_test_connector(() => { + assert.ok(!this.test_accept()); }); }); QUnit.test("can accept already connected inputs if input is multiple", function(assert) { - var self = this; this.multiple(); - this.with_test_connector(function() { - assert.ok(self.test_accept()); + this.with_test_connector(() => { + assert.ok(this.test_accept()); }); }); QUnit.test("cannot accept already connected inputs if input is multiple but datatypes don't match", function(assert) { var other = { node: {}, datatypes: ["binary"] }; // binary is not txt - var self = this; this.multiple(); - this.with_test_connector(function() { - assert.ok(!self.test_accept(other)); + this.with_test_connector(() => { + assert.ok(!this.test_accept(other)); }); }); QUnit.test("can accept list collection for multiple input parameters if datatypes match", function(assert) { - var self = this; this.multiple(); - assert.ok(self.test_accept()); + assert.ok(this.test_accept()); }); QUnit.test("can accept list collection for empty multiple inputs", function(assert) { @@ -212,9 +208,8 @@ QUnit.test("can accept list collection for empty multiple inputs", function(asse return new Terminals.CollectionTypeDescription("list"); } }; - var self = this; this.multiple(); - assert.ok(self.test_accept(other)); + assert.ok(this.test_accept(other)); }); QUnit.test("cannot accept list collection for multiple input if collection already connected", function(assert) { @@ -225,10 +220,9 @@ QUnit.test("cannot accept list collection for multiple input if collection alrea return new Terminals.CollectionTypeDescription("list"); } }; - var self = this; this.multiple(); - this.with_test_connector(function() { - assert.ok(!self.test_accept(other)); + this.with_test_connector(() => { + assert.ok(!this.test_accept(other)); }); }); @@ -304,9 +298,7 @@ QUnit.module("Input collection terminal model test", { }); QUnit.test("Collection output can connect to same collection input type", function(assert) { - var self = this; - var inputTerminal = self.input_terminal; - assert.ok(inputTerminal); + var inputTerminal = this.input_terminal; var outputTerminal = new Terminals.OutputCollectionTerminal({ datatypes: "txt", collection_type: "list" @@ -330,8 +322,7 @@ QUnit.test("Optiona collection output can not connect to required collection inp }); QUnit.test("Collection output cannot connect to different collection input type", function(assert) { - var self = this; - var inputTerminal = self.input_terminal; + var inputTerminal = this.input_terminal; var outputTerminal = new Terminals.OutputCollectionTerminal({ datatypes: "txt", collection_type: "paired" From 383034b9d7a9240cb5aaff1f5e64f0dc72a22ccc Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 11 Dec 2019 11:27:20 -0500 Subject: [PATCH 16/16] Stylistic improvements to workflow editor JS. --- .../qunit/tests/workflow_editor_tests.js | 120 ++++++++---------- 1 file changed, 56 insertions(+), 64 deletions(-) diff --git a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js index 225738042da0..e2a9d6b9c3c8 100644 --- a/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js +++ b/client/galaxy/scripts/qunit/tests/workflow_editor_tests.js @@ -160,7 +160,6 @@ QUnit.test("multiple inputs can accept optional outputs regardless", function(as // Galaxy multiple inputs have an optional field but it is hard to resolve that // completely until runtime. var other = { node: {}, datatypes: ["input"], optional: true }; - var self = this; this.multiple(); assert.ok(this.test_accept(other)); }); @@ -229,8 +228,8 @@ QUnit.test("cannot accept list collection for multiple input if collection alrea QUnit.module("Connector test", {}); QUnit.test("connects only if both valid handles", function(assert) { - var input = { connect: sinon.spy() }; - var output = { connect: sinon.spy() }; + const input = { connect: sinon.spy() }; + const output = { connect: sinon.spy() }; new Connector(input, null); new Connector(null, output); // Not attempts to connect... @@ -242,9 +241,9 @@ QUnit.test("connects only if both valid handles", function(assert) { }); QUnit.test("default attributes", function(assert) { - var input = { connect: sinon.spy() }; - var output = { connect: sinon.spy() }; - var connector = new Connector(input, output); + const input = { connect: sinon.spy() }; + const output = { connect: sinon.spy() }; + const connector = new Connector(input, output); assert.equal(connector.dragging, false); assert.equal(connector.canvas, null); assert.equal(connector.inner_color, "#FFFFFF"); @@ -252,31 +251,31 @@ QUnit.test("default attributes", function(assert) { }); QUnit.test("destroy", function(assert) { - var input = { connect: sinon.spy(), disconnect: sinon.spy() }; - var output = { connect: sinon.spy(), disconnect: sinon.spy() }; - var connector = new Connector(input, output); + const input = { connect: sinon.spy(), disconnect: sinon.spy() }; + const output = { connect: sinon.spy(), disconnect: sinon.spy() }; + const connector = new Connector(input, output); connector.destroy(); assert.ok(input.disconnect.called); assert.ok(output.disconnect.called); }); QUnit.test("initial redraw", function(assert) { - var input = { + const input = { connect: sinon.spy(), element: $("
"), isMappedOver: function() { return false; } }; - var output = { + const output = { connect: sinon.spy(), element: $("
"), isMappedOver: function() { return false; } }; - var connector = new Connector(input, output); - var n = $("#canvas-container").find("canvas").length; + const connector = new Connector(input, output); + const n = $("#canvas-container").find("canvas").length; connector.redraw(); // Ensure canvas gets set assert.ok(connector.canvas); @@ -298,8 +297,8 @@ QUnit.module("Input collection terminal model test", { }); QUnit.test("Collection output can connect to same collection input type", function(assert) { - var inputTerminal = this.input_terminal; - var outputTerminal = new Terminals.OutputCollectionTerminal({ + const inputTerminal = this.input_terminal; + const outputTerminal = new Terminals.OutputCollectionTerminal({ datatypes: "txt", collection_type: "list" }); @@ -310,9 +309,9 @@ QUnit.test("Collection output can connect to same collection input type", functi ); }); -QUnit.test("Optiona collection output can not connect to required collection input", function(assert) { - var inputTerminal = this.input_terminal; - var outputTerminal = new Terminals.OutputCollectionTerminal({ +QUnit.test("Optional collection output can not connect to required collection input", function(assert) { + const inputTerminal = this.input_terminal; + const outputTerminal = new Terminals.OutputCollectionTerminal({ datatypes: "txt", collection_type: "list", optional: true @@ -349,13 +348,13 @@ QUnit.module("Node unit test", { return $(this.node.element.find(selector)); }, expect_workflow_node_changed: function(assert, f) { - var node = this.node; - var node_changed_spy = sinon.spy(this.app.workflow, "node_changed"); + const node = this.node; + const node_changed_spy = sinon.spy(this.app.workflow, "node_changed"); f(); assert.ok(node_changed_spy.calledWith(node)); }, init_field_data_simple: function(option_overrides) { - var data = Utils.merge(option_overrides, { + const data = Utils.merge(option_overrides, { inputs: [{ name: "input1", extensions: ["data"] }], outputs: [{ name: "output1", extensions: ["data"] }], label: null @@ -363,7 +362,7 @@ QUnit.module("Node unit test", { this.node.init_field_data(data); }, update_field_data_with_new_input: function(option_overrides) { - var new_data = Utils.merge(option_overrides, { + const new_data = Utils.merge(option_overrides, { inputs: [{ name: "input1", extensions: ["data"] }, { name: "extra_0|input1", extensions: ["data"] }], outputs: [{ name: "output1", extensions: ["data"] }], post_job_actions: "{}", @@ -426,22 +425,20 @@ QUnit.test("init_field_data properties", function(assert) { }); QUnit.test("init_field_data data", function(assert) { - var test = this; - this.expect_workflow_node_changed(assert, function() { + this.expect_workflow_node_changed(assert, () => { // pre-init not tool form body... - assert.equal(test.$(".output-terminal").length, 0); - assert.equal(test.$(".input-terminal").length, 0); - assert.equal(test.$(".rule").length, 0); - test.init_field_data_simple(); + assert.equal(this.$(".output-terminal").length, 0); + assert.equal(this.$(".input-terminal").length, 0); + assert.equal(this.$(".rule").length, 0); + this.init_field_data_simple(); // After init tool form should have three "rows"/divs - , inputs div, one output, and rule... - assert.equal(test.$(".output-terminal").length, 1); - assert.equal(test.$(".input-terminal").length, 1); - assert.equal(test.$(".rule").length, 1); - assert.equal(test.$(".toolFormBody").children().length, 3); - assert.equal(test.$(".nodeTitle").text(), "newnode"); + assert.equal(this.$(".output-terminal").length, 1); + assert.equal(this.$(".input-terminal").length, 1); + assert.equal(this.$(".rule").length, 1); + assert.equal(this.$(".toolFormBody").children().length, 3); + assert.equal(this.$(".nodeTitle").text(), "newnode"); assert.ok( - test - .$(".toolFormTitle") + this.$(".toolFormTitle") .find("i") .hasClass("fa-wrench") ); @@ -449,62 +446,58 @@ QUnit.test("init_field_data data", function(assert) { }); QUnit.test("node title behavior", function(assert) { - var test = this; - this.expect_workflow_node_changed(assert, function() { + this.expect_workflow_node_changed(assert, () => { // Node created with name newnode - assert.equal(test.$(".nodeTitle").text(), "newnode"); + assert.equal(this.$(".nodeTitle").text(), "newnode"); // init_field_data_simple doesn't change label, so it should // remain original name. - test.init_field_data_simple(); - assert.equal(test.$(".nodeTitle").text(), "newnode"); + this.init_field_data_simple(); + assert.equal(this.$(".nodeTitle").text(), "newnode"); // Despite awkward name, update does change the label... - test.update_field_data_with_new_input(); - assert.equal(test.$(".nodeTitle").text(), "New Label"); + this.update_field_data_with_new_input(); + assert.equal(this.$(".nodeTitle").text(), "New Label"); }); }); QUnit.test("update_field_data updated data inputs and outputs", function(assert) { - var test = this; - this.expect_workflow_node_changed(assert, function() { + this.expect_workflow_node_changed(assert, () => { // Call init with one input and output. - test.init_field_data_simple(); + this.init_field_data_simple(); - test.update_field_data_with_new_input(); + this.update_field_data_with_new_input(); // Now there are 2 inputs... - assert.equal(test.$(".input-terminal").length, 2); - assert.equal(test.$(".output-terminal").length, 1); - assert.equal(test.$(".rule").length, 1); + assert.equal(this.$(".input-terminal").length, 2); + assert.equal(this.$(".output-terminal").length, 1); + assert.equal(this.$(".rule").length, 1); }); }); QUnit.test("update_field_data preserves connectors", function(assert) { - var test = this; var node = this.node; - this.expect_workflow_node_changed(assert, function() { + this.expect_workflow_node_changed(assert, () => { // Call init with one input and output. - test.init_field_data_simple(); + this.init_field_data_simple(); var connector = new Connector(); var old_input_terminal = node.input_terminals.input1; old_input_terminal.connectors.push(connector); // Update node, make sure connector still the same... - test.update_field_data_with_new_input(); + this.update_field_data_with_new_input(); var new_input_terminal = node.input_terminals.input1; assert.equal(connector, new_input_terminal.connectors[0]); // Update a second time, make sure connector still the same... - test.update_field_data_with_new_input(); + this.update_field_data_with_new_input(); new_input_terminal = node.input_terminals.input1; assert.equal(connector, new_input_terminal.connectors[0]); }); }); QUnit.test("update_field_data destroys old terminals", function(assert) { - var test = this; var node = this.node; - this.expect_workflow_node_changed(assert, function() { + this.expect_workflow_node_changed(assert, () => { var data = { inputs: [{ name: "input1", extensions: ["data"] }, { name: "willDisappear", extensions: ["data"] }], outputs: [{ name: "output1", extensions: ["data"] }] @@ -513,7 +506,7 @@ QUnit.test("update_field_data destroys old terminals", function(assert) { var old_input_terminal = node.input_terminals.willDisappear; var destroy_spy = sinon.spy(old_input_terminal, "destroy"); // Update - test.update_field_data_with_new_input(); + this.update_field_data_with_new_input(); assert.ok(destroy_spy.called); }); }); @@ -721,7 +714,6 @@ QUnit.test("terminal element", function(assert) { assert.equal(el.className, "terminal input-terminal"); }); -// global OutputTerminalView QUnit.module("Output terminal view", { beforeEach: function() { this.node = { output_terminals: [] }; @@ -883,9 +875,9 @@ QUnit.module("terminal mapping logic", { if (!("extensions" in input)) { input["extensions"] = ["data"]; } - var inputEl = $("
")[0]; - var inputTerminal = new Terminals.InputCollectionTerminal({ element: inputEl, input: input }); - var inputTerminalMapping = new Terminals.TerminalMapping({ terminal: inputTerminal }); + const inputEl = $("
")[0]; + const inputTerminal = new Terminals.InputCollectionTerminal({ element: inputEl, input: input }); + new Terminals.TerminalMapping({ terminal: inputTerminal }); inputTerminal.node = node; return inputTerminal; }, @@ -895,9 +887,9 @@ QUnit.module("terminal mapping logic", { if (!("extensions" in output)) { output["extensions"] = ["data"]; } - var outputEl = $("
")[0]; - var outputTerminal = new Terminals.OutputTerminal({ element: outputEl, datatypes: output.extensions }); - var outputTerminalMapping = new Terminals.TerminalMapping({ terminal: outputTerminal }); + const outputEl = $("
")[0]; + const outputTerminal = new Terminals.OutputTerminal({ element: outputEl, datatypes: output.extensions }); + new Terminals.TerminalMapping({ terminal: outputTerminal }); outputTerminal.node = node; if (mapOver) { outputTerminal.setMapOver(new Terminals.CollectionTypeDescription(mapOver));