Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve error message when creating a DictConfig from a bad dataclass #701

Merged
merged 3 commits into from
Apr 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/697.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve error message when creating a config from a Structured Config that fails type validation
23 changes: 16 additions & 7 deletions omegaconf/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
ConfigIndexError,
ConfigTypeError,
ConfigValueError,
GrammarParseError,
OmegaConfBaseException,
ValidationError,
)
Expand Down Expand Up @@ -253,12 +254,15 @@ def get_attr_data(obj: Any, allow_objects: Optional[bool] = None) -> Dict[str, A
from omegaconf.omegaconf import OmegaConf, _maybe_wrap

flags = {"allow_objects": allow_objects} if allow_objects is not None else {}
dummy_parent = OmegaConf.create(flags=flags)

from omegaconf import MISSING

d = {}
is_type = isinstance(obj, type)
obj_type = obj if is_type else type(obj)
dummy_parent = OmegaConf.create({}, flags=flags)
dummy_parent._metadata.object_type = obj_type

for name, attrib in attr.fields_dict(obj_type).items():
is_optional, type_ = _resolve_optional(attrib.type)
type_ = _resolve_forward(type_, obj.__module__)
Expand All @@ -282,8 +286,10 @@ def get_attr_data(obj: Any, allow_objects: Optional[bool] = None) -> Dict[str, A
value=value,
parent=dummy_parent,
)
except ValidationError as ex:
format_and_raise(node=None, key=name, value=value, cause=ex, msg=str(ex))
except (ValidationError, GrammarParseError) as ex:
format_and_raise(
node=dummy_parent, key=name, value=value, cause=ex, msg=str(ex)
)
d[name]._set_parent(None)
dict_subclass_data = extract_dict_subclass_data(obj=obj, parent=dummy_parent)
if dict_subclass_data is not None:
Expand All @@ -301,9 +307,10 @@ def get_dataclass_data(
from omegaconf.omegaconf import MISSING, OmegaConf, _maybe_wrap

flags = {"allow_objects": allow_objects} if allow_objects is not None else {}
dummy_parent = OmegaConf.create({}, flags=flags)
d = {}
obj_type = get_type_of(obj)
dummy_parent = OmegaConf.create({}, flags=flags)
dummy_parent._metadata.object_type = obj_type
resolved_hints = get_type_hints(obj_type)
for field in dataclasses.fields(obj):
name = field.name
Expand Down Expand Up @@ -333,8 +340,10 @@ def get_dataclass_data(
value=value,
parent=dummy_parent,
)
except ValidationError as ex:
format_and_raise(node=None, key=name, value=value, cause=ex, msg=str(ex))
except (ValidationError, GrammarParseError) as ex:
format_and_raise(
node=dummy_parent, key=name, value=value, cause=ex, msg=str(ex)
)
d[name]._set_parent(None)
dict_subclass_data = extract_dict_subclass_data(obj=obj, parent=dummy_parent)
if dict_subclass_data is not None:
Expand Down Expand Up @@ -709,7 +718,7 @@ def format_and_raise(

child_node: Optional[Node] = None
if node is None:
full_key = ""
full_key = key if key is not None else ""
object_type = None
ref_type = None
ref_type_str = None
Expand Down
2 changes: 1 addition & 1 deletion omegaconf/dictconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def __init__(
self.__dict__["_metadata"] = metadata
self._set_value(content, flags=flags)
except Exception as ex:
format_and_raise(node=None, key=None, value=None, cause=ex, msg=str(ex))
format_and_raise(node=None, key=key, value=None, cause=ex, msg=str(ex))

def __deepcopy__(self, memo: Dict[int, Any]) -> "DictConfig":
res = DictConfig(None)
Expand Down
2 changes: 1 addition & 1 deletion omegaconf/listconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def __init__(
self.__dict__["_content"] = None
self._set_value(value=content, flags=flags)
except Exception as ex:
format_and_raise(node=None, key=None, value=None, cause=ex, msg=str(ex))
format_and_raise(node=None, key=key, value=None, cause=ex, msg=str(ex))

def _validate_get(self, key: Any, value: Any = None) -> None:
if not isinstance(key, (int, slice)):
Expand Down
10 changes: 10 additions & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ class UnionError:
x: Union[int, str] = 10


@dataclass
class StructuredWithBadDict:
foo: Dict[str, str] = 123 # type: ignore


@dataclass
class StructuredWithBadList:
foo: List[str] = 123 # type: ignore


@dataclass
class MissingList:
list: List[str] = MISSING
Expand Down
36 changes: 32 additions & 4 deletions tests/test_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
Package,
Plugin,
Str2Int,
StructuredWithBadDict,
StructuredWithBadList,
StructuredWithMissing,
SubscriptedDict,
UnionError,
Expand Down Expand Up @@ -592,7 +594,9 @@ def finalize(self, cfg: Any) -> None:
exception_type=ValidationError,
msg="Non optional field cannot be assigned None",
key="foo",
full_key="",
full_key="foo",
parent_node=lambda _: {},
object_type=NotOptionalInt,
),
id="dict:create_none_optional_with_none",
),
Expand All @@ -603,7 +607,9 @@ def finalize(self, cfg: Any) -> None:
exception_type=ValidationError,
msg="Non optional field cannot be assigned None",
key="foo",
full_key="",
full_key="foo",
parent_node=lambda _: {},
object_type=NotOptionalInt,
),
id="dict:create:not_optional_int_field_with_none",
),
Expand Down Expand Up @@ -641,7 +647,9 @@ def finalize(self, cfg: Any) -> None:
exception_type=ValidationError,
msg="Value 'x' could not be converted to Integer",
key="foo",
full_key="",
full_key="foo",
parent_node=lambda _: {},
object_type=ConcretePlugin.FoobarParams,
),
id="structured:create_with_invalid_value",
),
Expand Down Expand Up @@ -782,11 +790,31 @@ def finalize(self, cfg: Any) -> None:
msg="Value 'qux' could not be converted to Integer",
object_type=None,
key="bar",
full_key="",
full_key="bar",
parent_node=lambda cfg: None,
),
id="structured,Dict_subclass:bad_value_type",
),
param(
Expected(
create=lambda: None,
op=lambda _: OmegaConf.structured(StructuredWithBadDict),
exception_type=ValidationError,
msg="Cannot assign int to Dict[str, str]",
key="foo",
),
id="structured,bad_default_value_for_dict",
),
param(
Expected(
create=lambda: None,
op=lambda _: OmegaConf.structured(StructuredWithBadList),
exception_type=ValidationError,
msg="Invalid value assigned: int is not a ListConfig, list or tuple.",
key="foo",
),
id="structured,bad_default_value_for_list",
),
##############
# ListConfig #
##############
Expand Down