Skip to content

Commit

Permalink
simplified and added additional test cases
Browse files Browse the repository at this point in the history
  • Loading branch information
omry committed Jan 27, 2021
1 parent a480938 commit 6b67d96
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 12 deletions.
1 change: 1 addition & 0 deletions news/496.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix merge when element type is a Structured Config
11 changes: 4 additions & 7 deletions omegaconf/basecontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,15 +349,12 @@ def expand(node: Container) -> None:
dest_node = dest._get_node(key)

if (
is_structured_config(dest._metadata.element_type)
dest_node is None
and is_structured_config(dest._metadata.element_type)
and not missing_src_value
):
if OmegaConf.is_none(dest, key) or OmegaConf.is_missing(dest, key):
dest[key] = DictConfig(
content=dest._metadata.element_type, parent=dest
)
else:
dest[key] = DictConfig(content=dest_node, parent=dest)
# merging into a new node. Use element_type as a base
dest[key] = DictConfig(content=dest._metadata.element_type, parent=dest)
dest_node = dest._get_node(key)

if dest_node is not None:
Expand Down
29 changes: 24 additions & 5 deletions tests/test_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
@pytest.mark.parametrize(
("merge_function", "input_unchanged"),
[
(OmegaConf.merge, True),
(OmegaConf.unsafe_merge, False),
pytest.param(OmegaConf.merge, True, id="merge"),
pytest.param(OmegaConf.unsafe_merge, False, id="unsafe_merge"),
],
)
@pytest.mark.parametrize(
Expand Down Expand Up @@ -181,23 +181,42 @@
id="inter:node_over_node_interpolation",
),
# Structured configs
(
pytest.param(
(DictConfig({}, element_type=User), {"user007": {"age": 99}}),
{"user007": {"name": "???", "age": 99}},
id="dict:merge_into_sc_element_type:expanding_new_element",
),
(
pytest.param(
(
DictConfig({"user007": "???"}, element_type=User),
{"user007": {"age": 99}},
),
{"user007": {"name": "???", "age": 99}},
id="dict:merge_into_sc_element_type:into_missing_element",
),
(
pytest.param(
(
DictConfig({"user007": User("bond", 7)}, element_type=User),
{"user007": {"age": 99}},
),
{"user007": {"name": "bond", "age": 99}},
id="dict:merge_into_sc_element_type:merging_with_existing_element",
),
pytest.param(
(
DictConfig({"user007": "???"}, element_type=User),
{"user007": {"age": 99}},
),
{"user007": {"name": "???", "age": 99}},
id="dict:merge_into_sc_element_type:merging_into_missing",
),
pytest.param(
(
DictConfig({"user007": None}, element_type=User),
{"user007": {"age": 99}},
),
{"user007": {"name": "???", "age": 99}},
id="dict:merge_into_sc_element_type:merging_into_none",
),
(({"user": User}, {}), {"user": User(name=MISSING, age=MISSING)}),
(({"user": User}, {"user": {}}), {"user": User(name=MISSING, age=MISSING)}),
Expand Down

0 comments on commit 6b67d96

Please sign in to comment.