diff --git a/news/496.bugfix b/news/496.bugfix new file mode 100644 index 000000000..bb74ba65c --- /dev/null +++ b/news/496.bugfix @@ -0,0 +1 @@ +Fix merge when element type is a Structured Config diff --git a/omegaconf/basecontainer.py b/omegaconf/basecontainer.py index 12669a287..0767533c3 100644 --- a/omegaconf/basecontainer.py +++ b/omegaconf/basecontainer.py @@ -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: diff --git a/tests/test_merge.py b/tests/test_merge.py index 9e117fa6c..8aabe45db 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -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( @@ -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)}),