diff --git a/news/743.api_change b/news/743.api_change new file mode 100644 index 000000000..bfd494107 --- /dev/null +++ b/news/743.api_change @@ -0,0 +1 @@ +Improved error message when assigning an invalid value to int or float config nodes diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index d3dafe9b8..13495c723 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -774,7 +774,7 @@ def format_and_raise( KEY=key, FULL_KEY=full_key, VALUE=value, - VALUE_TYPE=f"{type(value).__name__}", + VALUE_TYPE=type_str(type(value), include_module_name=True), KEY_TYPE=f"{type(key).__name__}", ) @@ -821,7 +821,7 @@ def format_and_raise( _raise(ex, cause) -def type_str(t: Any) -> str: +def type_str(t: Any, include_module_name: bool = False) -> str: is_optional, t = _resolve_optional(t) if t is None: return type(t).__name__ @@ -848,16 +848,31 @@ def type_str(t: Any) -> str: else: if t._name is None: if t.__origin__ is not None: - name = type_str(t.__origin__) + name = type_str( + t.__origin__, include_module_name=include_module_name + ) else: name = str(t._name) args = getattr(t, "__args__", None) if args is not None: - args = ", ".join([type_str(t) for t in t.__args__]) + args = ", ".join( + [type_str(t, include_module_name=include_module_name) for t in t.__args__] + ) ret = f"{name}[{args}]" else: ret = name + if include_module_name: + if ( + hasattr(t, "__module__") + and t.__module__ != "builtins" + and t.__module__ != "typing" + and not t.__module__.startswith("omegaconf.") + ): + module_prefix = t.__module__ + "." + else: + module_prefix = "" + ret = module_prefix + ret if is_optional: return f"Optional[{ret}]" else: diff --git a/omegaconf/nodes.py b/omegaconf/nodes.py index d0f5af81e..b844f5e8b 100644 --- a/omegaconf/nodes.py +++ b/omegaconf/nodes.py @@ -203,7 +203,9 @@ def _validate_and_convert_impl(self, value: Any) -> int: else: raise ValueError() except ValueError: - raise ValidationError("Value '$VALUE' could not be converted to Integer") + raise ValidationError( + "Value '$VALUE' of type '$VALUE_TYPE' could not be converted to Integer" + ) return val def __deepcopy__(self, memo: Dict[int, Any]) -> "IntegerNode": @@ -240,7 +242,9 @@ def _validate_and_convert_impl(self, value: Any) -> float: else: raise ValueError() except ValueError: - raise ValidationError("Value '$VALUE' could not be converted to Float") + raise ValidationError( + "Value '$VALUE' of type '$VALUE_TYPE' could not be converted to Float" + ) def __eq__(self, other: Any) -> bool: if isinstance(other, ValueNode): diff --git a/tests/interpolation/test_interpolation.py b/tests/interpolation/test_interpolation.py index e769fb5c4..6f18ecd79 100644 --- a/tests/interpolation/test_interpolation.py +++ b/tests/interpolation/test_interpolation.py @@ -332,7 +332,7 @@ def drop_last(s: str) -> str: match=re.escape( dedent( """\ - Value 'seven' could not be converted to Integer + Value 'seven' of type 'str' could not be converted to Integer full_key: age """ ) @@ -345,7 +345,9 @@ def drop_last(s: str) -> str: "age", raises( InterpolationValidationError, - match=re.escape("'Bond' could not be converted to Integer"), + match=re.escape( + "'Bond' of type 'str' could not be converted to Integer" + ), ), id="type_mismatch_node_interpolation", ), diff --git a/tests/structured_conf/test_structured_basic.py b/tests/structured_conf/test_structured_basic.py index f2b715a47..7fe3b3887 100644 --- a/tests/structured_conf/test_structured_basic.py +++ b/tests/structured_conf/test_structured_basic.py @@ -51,7 +51,9 @@ def test_error_on_non_structured_nested_config_class(self, module: Any) -> None: def test_error_on_creation_with_bad_value_type(self, module: Any) -> None: with raises( ValidationError, - match=re.escape("Value 'seven' could not be converted to Integer"), + match=re.escape( + "Value 'seven' of type 'str' could not be converted to Integer" + ), ): OmegaConf.structured(module.User(age="seven")) diff --git a/tests/test_basic_ops_list.py b/tests/test_basic_ops_list.py index 7bdcf3fd8..950ce7253 100644 --- a/tests/test_basic_ops_list.py +++ b/tests/test_basic_ops_list.py @@ -331,7 +331,9 @@ def test_list_append() -> None: "foo", raises( ValidationError, - match=re.escape("Value 'foo' could not be converted to Integer"), + match=re.escape( + "Value 'foo' of type 'str' could not be converted to Integer" + ), ), id="append_str_to_list[int]", ), diff --git a/tests/test_errors.py b/tests/test_errors.py index 92ee255cc..f86077649 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -9,6 +9,7 @@ import tests from omegaconf import ( DictConfig, + FloatNode, IntegerNode, ListConfig, OmegaConf, @@ -113,7 +114,7 @@ def finalize(self, cfg: Any) -> None: create=lambda: OmegaConf.structured(StructuredWithMissing), op=lambda cfg: OmegaConf.update(cfg, "num", "hello"), exception_type=ValidationError, - msg="Value 'hello' could not be converted to Integer", + msg="Value 'hello' of type 'str' could not be converted to Integer", parent_node=lambda cfg: cfg, child_node=lambda cfg: cfg._get_node("num"), object_type=StructuredWithMissing, @@ -362,7 +363,7 @@ def finalize(self, cfg: Any) -> None: create=lambda: OmegaConf.structured(ConcretePlugin), op=lambda cfg: cfg.params.__setattr__("foo", "bar"), exception_type=ValidationError, - msg="Value 'bar' could not be converted to Integer", + msg="Value 'bar' of type 'str' could not be converted to Integer", key="foo", full_key="params.foo", object_type=ConcretePlugin.FoobarParams, @@ -484,7 +485,7 @@ def finalize(self, cfg: Any) -> None: create=lambda: OmegaConf.structured(ConcretePlugin), op=lambda cfg: OmegaConf.merge(cfg, {"params": {"foo": "bar"}}), exception_type=ValidationError, - msg="Value 'bar' could not be converted to Integer", + msg="Value 'bar' of type 'str' could not be converted to Integer", key="foo", full_key="params.foo", object_type=ConcretePlugin.FoobarParams, @@ -646,13 +647,37 @@ def finalize(self, cfg: Any) -> None: ConcretePlugin(params=ConcretePlugin.FoobarParams(foo="x")) # type: ignore ), exception_type=ValidationError, - msg="Value 'x' could not be converted to Integer", + msg="Value 'x' of type 'str' could not be converted to Integer", key="foo", full_key="foo", parent_node=lambda _: {}, object_type=ConcretePlugin.FoobarParams, ), - id="structured:create_with_invalid_value", + id="structured:create_with_invalid_value,int", + ), + param( + Expected( + create=lambda: DictConfig({"bar": FloatNode(123.456)}), + op=lambda cfg: cfg.__setattr__("bar", "x"), + exception_type=ValidationError, + msg="Value 'x' of type 'str' could not be converted to Float", + key="bar", + full_key="bar", + child_node=lambda cfg: cfg._get_node("bar"), + ), + id="typed_DictConfig:assign_with_invalid_value,float", + ), + param( + Expected( + create=lambda: DictConfig({"bar": FloatNode(123.456)}), + op=lambda cfg: cfg.__setattr__("bar", Color.BLUE), + exception_type=ValidationError, + msg="Value 'Color.BLUE' of type 'tests.Color' could not be converted to Float", + key="bar", + full_key="bar", + child_node=lambda cfg: cfg._get_node("bar"), + ), + id="typed_DictConfig:assign_with_invalid_value,full_module_in_error", ), param( Expected( @@ -694,7 +719,7 @@ def finalize(self, cfg: Any) -> None: ), op=lambda cfg: cfg.__setitem__("baz", "fail"), exception_type=ValidationError, - msg="Value 'fail' could not be converted to Integer", + msg="Value 'fail' of type 'str' could not be converted to Integer", key="baz", ), id="DictConfig[str,int]:assigned_str_value", @@ -1102,7 +1127,7 @@ def finalize(self, cfg: Any) -> None: create=lambda: ListConfig(element_type=int, content=[1, 2, 3]), op=lambda cfg: cfg.__setitem__(0, "foo"), exception_type=ValidationError, - msg="Value 'foo' could not be converted to Integer", + msg="Value 'foo' of type 'str' could not be converted to Integer", key=0, full_key="[0]", child_node=lambda cfg: cfg[0], @@ -1117,7 +1142,7 @@ def finalize(self, cfg: Any) -> None: ), op=lambda cfg: cfg.__setitem__(0, "foo"), exception_type=ValidationError, - msg="Value 'foo' could not be converted to Integer", + msg="Value 'foo' of type 'str' could not be converted to Integer", key=0, full_key="[0]", child_node=lambda cfg: cfg[0], @@ -1464,7 +1489,7 @@ def test_dict_subclass_error() -> None: src["bar"] = "qux" # type: ignore with raises( ValidationError, - match=re.escape("Value 'qux' could not be converted to Integer"), + match=re.escape("Value 'qux' of type 'str' could not be converted to Integer"), ) as einfo: with warns_dict_subclass_deprecated(Str2Int): OmegaConf.structured(src) diff --git a/tests/test_utils.py b/tests/test_utils.py index 50a6e8e33..ea0a9632e 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -366,32 +366,56 @@ def test_is_primitive_type(type_: Any, is_primitive: bool) -> None: @mark.parametrize("optional", [False, True]) @mark.parametrize( - "type_, expected", + "type_, include_module_name, expected", [ - (int, "int"), - (bool, "bool"), - (float, "float"), - (str, "str"), - (Color, "Color"), - (DictConfig, "DictConfig"), - (ListConfig, "ListConfig"), - (Dict[str, str], "Dict[str, str]"), - (Dict[Color, int], "Dict[Color, int]"), - (Dict[str, Plugin], "Dict[str, Plugin]"), - (Dict[str, List[Plugin]], "Dict[str, List[Plugin]]"), - (List[str], "List[str]"), - (List[Color], "List[Color]"), - (List[Dict[str, Color]], "List[Dict[str, Color]]"), - (Tuple[str], "Tuple[str]"), - (Tuple[str, int], "Tuple[str, int]"), - (Tuple[float, ...], "Tuple[float, ...]"), + (int, False, "int"), + (int, True, "int"), + (bool, False, "bool"), + (bool, True, "bool"), + (float, False, "float"), + (float, True, "float"), + (str, False, "str"), + (str, True, "str"), + (Color, False, "Color"), + (Color, True, "tests.Color"), + (DictConfig, False, "DictConfig"), + (DictConfig, True, "DictConfig"), + (ListConfig, False, "ListConfig"), + (ListConfig, True, "ListConfig"), + (Dict[str, str], False, "Dict[str, str]"), + (Dict[str, str], True, "Dict[str, str]"), + (Dict[Color, int], False, "Dict[Color, int]"), + (Dict[Color, int], True, "Dict[tests.Color, int]"), + (Dict[str, Plugin], False, "Dict[str, Plugin]"), + (Dict[str, Plugin], True, "Dict[str, tests.Plugin]"), + (Dict[str, List[Plugin]], False, "Dict[str, List[Plugin]]"), + (Dict[str, List[Plugin]], True, "Dict[str, List[tests.Plugin]]"), + (List[str], False, "List[str]"), + (List[str], True, "List[str]"), + (List[Color], False, "List[Color]"), + (List[Color], True, "List[tests.Color]"), + (List[Dict[str, Color]], False, "List[Dict[str, Color]]"), + (List[Dict[str, Color]], True, "List[Dict[str, tests.Color]]"), + (Tuple[str], False, "Tuple[str]"), + (Tuple[str], True, "Tuple[str]"), + (Tuple[str, int], False, "Tuple[str, int]"), + (Tuple[str, int], True, "Tuple[str, int]"), + (Tuple[float, ...], False, "Tuple[float, ...]"), + (Tuple[float, ...], True, "Tuple[float, ...]"), ], ) -def test_type_str(type_: Any, expected: str, optional: bool) -> None: +def test_type_str( + type_: Any, include_module_name: bool, expected: str, optional: bool +) -> None: if optional: - assert _utils.type_str(Optional[type_]) == f"Optional[{expected}]" + assert ( + _utils.type_str(Optional[type_], include_module_name=include_module_name) + == f"Optional[{expected}]" + ) else: - assert _utils.type_str(type_) == expected + assert ( + _utils.type_str(type_, include_module_name=include_module_name) == expected + ) def test_type_str_ellipsis() -> None: