From dcec15a8526e94319a2f5b0e8f49b65fe6b5dcab Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 17 Jun 2019 17:34:59 +0200 Subject: [PATCH] include parameter name in exceptions - to simplify tool testing its of advantage to know which parameter caused a problem - for select parameters the illegal value and the list of legal values is added - also unified the text of the messages Co-Authored-By: Marius van den Beek --- lib/galaxy/tools/parameters/basic.py | 94 +++++++++++------------ test/unit/tools/test_select_parameters.py | 6 +- 2 files changed, 50 insertions(+), 50 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 74e4fa5d7b1..7db9dcd807f 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -239,9 +239,9 @@ class ToolParameter(Dictifiable): param_name = cls.parse_name(param) param_type = param.get('type') if not param_type: - raise ValueError("Tool parameter '%s' requires a 'type'" % (param_name)) + raise ValueError("parameter '%s' requires a 'type'" % (param_name)) elif param_type not in parameter_types: - raise ValueError("Tool parameter '%s' uses an unknown type '%s'" % (param_name, param_type)) + raise ValueError("parameter '%s' uses an unknown type '%s'" % (param_name, param_type)) else: return parameter_types[param_type](tool, param) @@ -253,7 +253,7 @@ class ToolParameter(Dictifiable): if argument: name = argument.lstrip('-') else: - raise ValueError("Tool parameter must specify a name.") + raise ValueError("parameter must specify a name.") return name @@ -315,7 +315,7 @@ class IntegerToolParameter(TextToolParameter): >>> type(p.from_json("_string", trans)) Traceback (most recent call last): ... - ValueError: An integer or workflow parameter e.g. ${name} is required + ValueError: parameter '_name': an integer or workflow parameter is required """ dict_collection_visible_keys = ToolParameter.dict_collection_visible_keys + ['min', 'max'] @@ -326,21 +326,21 @@ class IntegerToolParameter(TextToolParameter): try: int(self.value) except ValueError: - raise ValueError("An integer is required") + raise ValueError("parameter '%s': the attribute 'value' must be an integer" % self.name) elif self.value is None and not self.optional: - raise ValueError("The settings for the field named '%s' require a 'value' setting and optionally a default value which must be an integer" % self.name) + raise ValueError("parameter '%s': the attribute 'value' must be set for non optional parameters" % self.name) self.min = input_source.get('min') self.max = input_source.get('max') if self.min: try: self.min = int(self.min) except ValueError: - raise ValueError("An integer is required") + raise ValueError("parameter '%s': attribute 'min' must be an integer" % self.name) if self.max: try: self.max = int(self.max) except ValueError: - raise ValueError("An integer is required") + raise ValueError("parameter '%s': attribute 'max' must be an integer" % self.name) if self.min is not None or self.max is not None: self.validators.append(validation.InRangeValidator(None, self.min, self.max)) @@ -353,9 +353,9 @@ class IntegerToolParameter(TextToolParameter): if not value and self.optional: return "" if trans.workflow_building_mode is workflow_building_modes.ENABLED: - raise ValueError("An integer or workflow parameter e.g. ${name} is required") + raise ValueError("parameter '%s': an integer or workflow parameter is required" % self.name) else: - raise ValueError("An integer is required") + raise ValueError("parameter '%s': the attribute 'value' must be set for non optional parameters" % self.name) def to_python(self, value, app): try: @@ -388,7 +388,7 @@ class FloatToolParameter(TextToolParameter): >>> type(p.from_json("_string", trans)) Traceback (most recent call last): ... - ValueError: A real number or workflow parameter e.g. ${name} is required + ValueError: parameter '_name': an integer or workflow parameter is required """ dict_collection_visible_keys = ToolParameter.dict_collection_visible_keys + ['min', 'max'] @@ -401,19 +401,19 @@ class FloatToolParameter(TextToolParameter): try: float(self.value) except ValueError: - raise ValueError("A real number is required") + raise ValueError("parameter '%s': the attribute 'value' must be a real number" % self.name) elif self.value is None and not self.optional: - raise ValueError("The settings for this field require a 'value' setting and optionally a default value which must be a real number") + raise ValueError("parameter '%s': the attribute 'value' must be set for non optional parameters" % self.name) if self.min: try: self.min = float(self.min) except ValueError: - raise ValueError("A real number is required") + raise ValueError("parameter '%s': attribute 'min' must be a real number" % self.name) if self.max: try: self.max = float(self.max) except ValueError: - raise ValueError("A real number is required") + raise ValueError("parameter '%s': attribute 'max' must be a real number" % self.name) if self.min is not None or self.max is not None: self.validators.append(validation.InRangeValidator(None, self.min, self.max)) @@ -425,10 +425,10 @@ class FloatToolParameter(TextToolParameter): return value if not value and self.optional: return "" - if trans and trans.workflow_building_mode is workflow_building_modes.ENABLED: - raise ValueError("A real number or workflow parameter e.g. ${name} is required") + if trans.workflow_building_mode is workflow_building_modes.ENABLED: + raise ValueError("parameter '%s': an integer or workflow parameter is required" % self.name) else: - raise ValueError("A real number is required") + raise ValueError("parameter '%s': the attribute 'value' must be set for non optional parameters" % self.name) def to_python(self, value, app): try: @@ -867,20 +867,20 @@ class SelectToolParameter(ToolParameter): elif value is None: if self.optional: return None - raise ValueError("An invalid option was selected for %s, please verify." % (self.name)) + raise ValueError("parameter '%s': an invalid option (None) was selected, please verify" % self.name) elif not legal_values: if self.optional and self.tool.profile < 18.09: # Covers optional parameters with default values that reference other optional parameters. # These will have a value but no legal_values. return None - raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name) + raise ValueError("parameter '%s': requires a value, but no legal values defined" % self.name) if isinstance(value, list): if not self.multiple: - raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name) + raise ValueError("parameter '%s': multiple values provided but parameter is not expecting multiple values" % (self.name)) rval = [] for v in value: if v not in legal_values: - raise ValueError("An invalid option was selected for %s, %r, please verify." % (self.name, v)) + raise ValueError("parameter '%s': an invalid option (%r) was selected (valid options: %s)" % (self.name, v, ",".join(legal_values))) rval.append(v) return rval else: @@ -890,9 +890,9 @@ class SelectToolParameter(ToolParameter): if self.optional: return [] else: - raise ValueError("No option was selected for %s but input is not optional." % self.name) + raise ValueError("parameter '%s': no option was selected for non optional parameter" % (self.name)) if value not in legal_values and require_legal_value: - raise ValueError("An invalid option was selected for %s, %r, please verify." % (self.name, value)) + raise ValueError("parameter '%s': an invalid option (%r) was selected (valid options: %s)" % (self.name, value, ",".join(legal_values))) return value def to_param_dict_string(self, value, other_values={}): @@ -900,7 +900,7 @@ class SelectToolParameter(ToolParameter): return "None" if isinstance(value, list): if not self.multiple: - raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name) + raise ValueError("parameter '%s': multiple values provided but parameter is not expecting multiple values" % (self.name)) value = list(map(str, value)) else: value = str(value) @@ -1427,17 +1427,17 @@ class DrillDownSelectToolParameter(SelectToolParameter): elif value is None: if self.optional: return None - raise ValueError("An invalid option was selected for %s, please verify." % (self.name)) + raise ValueError("parameter '%s': an invalid option (%r) was selected" % (self.name, value)) elif not legal_values: - raise ValueError("Parameter %s requires a value, but has no legal values defined." % self.name) + raise ValueError("parameter '%s': requires a value, but no legal values defined" % (self.name)) if not isinstance(value, list): value = [value] if len(value) > 1 and not self.multiple: - raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name) + raise ValueError("parameter '%s': multiple values provided but parameter is not expecting multiple values" % (self.name)) rval = [] for val in value: if val not in legal_values: - raise ValueError("An invalid option was selected for %s, %r, please verify" % (self.name, val)) + raise ValueError("parameter '%s': an invalid option (%r) was selected (valid options: %s)" % (self.name, val, ",".join(legal_values))) rval.append(val) return rval @@ -1472,7 +1472,7 @@ class DrillDownSelectToolParameter(SelectToolParameter): options = get_options_list(val) rval.extend(options) if len(rval) > 1 and not self.multiple: - raise ValueError("Multiple values provided but parameter %s is not expecting multiple values." % self.name) + raise ValueError("parameter '%s': multiple values provided but parameter is not expecting multiple values" % (self.name)) rval = self.separator.join(rval) if self.tool is None or self.tool.options.sanitize: if self.sanitizer: @@ -1704,16 +1704,16 @@ class DataToolParameter(BaseDataToolParameter): try: self.min = int(self.min) except ValueError: - raise ValueError("An integer is required for min property.") + raise ValueError("parameter '%s': attribute 'min' must be an integer" % self.name) if self.max: try: self.max = int(self.max) except ValueError: - raise ValueError("An integer is required for max property.") + raise ValueError("parameter '%s': attribute 'max' must be an integer" % self.name) if not self.multiple and (self.min is not None): - raise ValueError("Cannot specify min property on single data parameter '%s'. Set multiple=\"true\" to enable this option." % self.name) + raise ValueError("parameter '%s': cannot specify 'min' property on single data parameter '%s'. Set multiple=\"true\" to enable this option" % self.name) if not self.multiple and (self.max is not None): - raise ValueError("Cannot specify max property on single data parameter '%s'. Set multiple=\"true\" to enable this option." % self.name) + raise ValueError("parameter '%s': cannot specify 'max' property on single data parameter '%s'. Set multiple=\"true\" to enable this option" % self.name) self.is_dynamic = True self._parse_options(input_source) # Load conversions required for the dataset input @@ -1723,14 +1723,14 @@ class DataToolParameter(BaseDataToolParameter): if self.datatypes_registry: conv_type = self.datatypes_registry.get_datatype_by_extension(conv_extension.lower()) if conv_type is None: - raise ValueError("Datatype class not found for extension '%s', which is used as 'type' attribute in conversion of data parameter '%s'" % (conv_type, self.name)) + raise ValueError("parameter '%s': datatype class not found for extension '%s', which is used as 'type' attribute in conversion of data parameter" % (self.name, conv_type)) self.conversions.append((name, conv_extension, [conv_type])) def from_json(self, value, trans, other_values={}): if trans.workflow_building_mode is workflow_building_modes.ENABLED or is_runtime_value(value): return None if not value and not self.optional: - raise ValueError("Specify a dataset of the required format / build for parameter %s." % self.name) + raise ValueError("parameter '%s': specify a dataset of the required format / build for parameter" % self.name) if value in [None, "None", '']: return None if isinstance(value, dict) and 'values' in value: @@ -1772,7 +1772,7 @@ class DataToolParameter(BaseDataToolParameter): if found_hdca: for val in rval: if not isinstance(val, trans.app.model.HistoryDatasetCollectionAssociation): - raise ValueError("If collections are supplied to multiple data input parameter, only collections may be used.") + raise ValueError("parameter '%s': if collections are supplied to multiple data input parameter, only collections may be used" % self.name) elif isinstance(value, trans.app.model.HistoryDatasetAssociation): rval = value elif isinstance(value, dict) and 'src' in value and 'id' in value: @@ -1801,20 +1801,20 @@ class DataToolParameter(BaseDataToolParameter): for v in values: if v: if hasattr(v, "deleted") and v.deleted: - raise ValueError("The previously selected dataset has been deleted.") + raise ValueError("parameter '%s': the previously selected dataset has been deleted." % self.name) elif hasattr(v, "dataset") and v.dataset.state in [galaxy.model.Dataset.states.ERROR, galaxy.model.Dataset.states.DISCARDED]: - raise ValueError("The previously selected dataset has entered an unusable state") + raise ValueError("parameter '%s': the previously selected dataset has entered an unusable state" % self.name) elif hasattr(v, "dataset"): match = dataset_matcher.hda_match(v) if match and match.implicit_conversion: v.implicit_conversion = True if not self.multiple: if len(values) > 1: - raise ValueError("More than one dataset supplied to single input dataset parameter.") + raise ValueError("parameter '%s': more than one dataset supplied to single input dataset parameter" % self.name) if len(values) > 0: rval = values[0] else: - raise ValueError("Invalid dataset supplied to single input dataset parameter.") + raise ValueError("parameter '%s': invalid dataset supplied to single input dataset parameter" % self.name) return rval def to_param_dict_string(self, value, other_values={}): @@ -1863,10 +1863,10 @@ class DataToolParameter(BaseDataToolParameter): if self.min is not None: if self.min > dataset_count: - raise ValueError("At least %d datasets are required." % self.min) + raise ValueError("At least %d datasets are required for %s" % (self.min, self.name)) if self.max is not None: if self.max < dataset_count: - raise ValueError("At most %d datasets are required." % self.max) + raise ValueError("At most %d datasets are required for %s" % (self.max, self.name)) def get_dependencies(self): """ @@ -2052,7 +2052,7 @@ class DataCollectionToolParameter(BaseDataToolParameter): if trans.workflow_building_mode is workflow_building_modes.ENABLED: return None if not value and not self.optional: - raise ValueError("Specify a dataset collection of the correct type.") + raise ValueError("parameter '%s': specify a dataset collection of the correct type" % self.name) if value in [None, "None"]: return None if isinstance(value, dict) and 'values' in value: @@ -2084,7 +2084,7 @@ class DataCollectionToolParameter(BaseDataToolParameter): rval = trans.sa_session.query(trans.app.model.HistoryDatasetCollectionAssociation).get(value) if rval and isinstance(rval, trans.app.model.HistoryDatasetCollectionAssociation): if rval.deleted: - raise ValueError("The previously selected dataset collection has been deleted") + raise ValueError("parameter '%s': the previously selected dataset collection has been deleted" % self.name) # TODO: Handle error states, implement error states ... return rval @@ -2242,10 +2242,10 @@ class LibraryDatasetToolParameter(ToolParameter): if lda is not None: lst.append(lda) elif validate: - raise ValueError("One of the selected library datasets is invalid or not available anymore.") + raise ValueError("parameter '%s': one of the selected library datasets is invalid or not available anymore" % self.name) if len(lst) == 0: if not self.optional and validate: - raise ValueError("Please select a valid library dataset.") + raise ValueError("parameter '%s': invalid library dataset selected" % self.name) return None else: return lst diff --git a/test/unit/tools/test_select_parameters.py b/test/unit/tools/test_select_parameters.py index 7e778c4602f..28cfe50e33d 100644 --- a/test/unit/tools/test_select_parameters.py +++ b/test/unit/tools/test_select_parameters.py @@ -11,7 +11,7 @@ class SelectToolParameterTestCase(BaseParameterTestCase): try: self.param.from_json("42", self.trans, {"input_bam": model.HistoryDatasetAssociation()}) except ValueError as err: - assert str(err) == "An invalid option was selected for my_name, '42', please verify." + assert str(err) == "parameter 'my_name': an invalid option ('42') was selected (valid options: ?)" return assert False @@ -20,7 +20,7 @@ class SelectToolParameterTestCase(BaseParameterTestCase): try: self.param.from_json("42", self.trans) except ValueError as err: - assert str(err) == "Parameter my_name requires a value, but has no legal values defined." + assert str(err) == "parameter 'my_name': requires a value, but no legal values defined" return assert False @@ -34,7 +34,7 @@ class SelectToolParameterTestCase(BaseParameterTestCase): try: self.param.from_json(model.HistoryDatasetAssociation(), self.trans, {"input_bam": None}) except ValueError as err: - assert str(err) == "Parameter my_name requires a value, but has no legal values defined." + assert str(err) == "parameter 'my_name': requires a value, but no legal values defined" return assert False