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.

Sign up for GitHub

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

in get_structured_config_data: check for dict subclass data #653

Merged
merged 8 commits into from
Apr 11, 2021
Merged

in get_structured_config_data: check for dict subclass data #653

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
74d4621
in get_structured_config_data: check for dict subclass data
Jasha10 Mar 30, 2021
9cff002
use correct module for _resolve_forward
Jasha10 Mar 30, 2021
d165997
coverage for `except ValidationError` code path
Jasha10 Mar 30, 2021
d30c7d7
refactor: extract function that gets dict subclass data
Jasha10 Apr 2, 2021
e6ed8a1
Use dict subclass with class field
Jasha10 Apr 3, 2021
c6e827c
Merge master to resolve conflicts
Jasha10 Apr 9, 2021
9f37ab1
test_errors.py: testcase for ValidationError message
Jasha10 Apr 9, 2021
8de6ab8
add news fragment
Jasha10 Apr 9, 2021
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
37 changes: 36 additions & 1 deletion omegaconf/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,23 @@ def get_attr_data(obj: Any, allow_objects: Optional[bool] = None) -> Dict[str, A
except ValidationError as ex:
format_and_raise(node=None, key=name, value=value, cause=ex, msg=str(ex))
d[name]._set_parent(None)
if is_dict_subclass(obj_type) and not isinstance(obj, type):
key_type, element_type = get_dict_key_value_types(obj_type)
for name, value in obj.items():
is_optional, type_ = _resolve_optional(element_type)
type_ = _resolve_forward(type_, element_type.__module__)
try:
d[name] = _maybe_wrap(
ref_type=type_,
is_optional=is_optional,
key=name,
value=value,
parent=dummy_parent,
)
except ValidationError as ex:
format_and_raise(
node=None, key=name, value=value, cause=ex, msg=str(ex)
)
return d


Expand All @@ -248,7 +265,8 @@ def get_dataclass_data(
flags = {"allow_objects": allow_objects} if allow_objects is not None else {}
dummy_parent = OmegaConf.create({}, flags=flags)
d = {}
resolved_hints = get_type_hints(get_type_of(obj))
obj_type = get_type_of(obj)
resolved_hints = get_type_hints(obj_type)
for field in dataclasses.fields(obj):
name = field.name
is_optional, type_ = _resolve_optional(resolved_hints[field.name])
Expand Down Expand Up @@ -280,6 +298,23 @@ def get_dataclass_data(
except ValidationError as ex:
format_and_raise(node=None, key=name, value=value, cause=ex, msg=str(ex))
d[name]._set_parent(None)
if is_dict_subclass(obj_type) and not isinstance(obj, type):
key_type, element_type = get_dict_key_value_types(obj_type)
for name, value in obj.items():
is_optional, type_ = _resolve_optional(element_type)
type_ = _resolve_forward(type_, element_type.__module__)
try:
d[name] = _maybe_wrap(
ref_type=type_,
is_optional=is_optional,
key=name,
value=value,
parent=dummy_parent,
)
except ValidationError as ex:
format_and_raise(
node=None, key=name, value=value, cause=ex, msg=str(ex)
)
return d


Expand Down
6 changes: 6 additions & 0 deletions tests/structured_conf/test_structured_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,12 @@ def test_str2str(self, module: Any) -> None:
with raises(KeyValidationError):
cfg[Color.RED]

def test_dict_subclass_data_preserved_upon_node_creation(self, module: Any) -> None:
src = module.DictSubclass.Str2Str()
src["baz"] = "qux"
cfg = OmegaConf.structured(src)
assert cfg.baz == "qux"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If run against the master branch, the above test raises ConfigAttributeError: Missing key baz.
This PR addresses the error by inspecting for dict subclass data within the get_attr_data and get_dataclass_data functions.


def test_str2str_as_sub_node(self, module: Any) -> None:
cfg = OmegaConf.create({"foo": module.DictSubclass.Str2Str})
assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Str2Str
Expand Down