Skip to content

Commit

Permalink
ListConfig append deepcopies input config nodes (#738)
Browse files Browse the repository at this point in the history
  • Loading branch information
omry authored Jun 3, 2021
1 parent 4dd8085 commit bc03bb3
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 0 deletions.
1 change: 1 addition & 0 deletions news/601.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ListConfig append now copies input config nodes
5 changes: 5 additions & 0 deletions omegaconf/listconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,11 @@ def append(self, item: Any) -> None:
index = len(self)
self._validate_set(key=index, value=item)

if isinstance(item, Node):
do_deepcopy = not self._get_flag("no_deepcopy_set_nodes")
if do_deepcopy:
item = copy.deepcopy(item)

node = _maybe_wrap(
ref_type=self.__dict__["_metadata"].element_type,
key=index,
Expand Down
15 changes: 15 additions & 0 deletions tests/test_basic_ops_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from omegaconf import (
MISSING,
AnyNode,
DictConfig,
DictKeyType,
ListConfig,
Expand Down Expand Up @@ -1133,3 +1134,17 @@ def test_dictconfig_creation_with_parent_flag(flag: str, data: Any) -> None:
parent._set_flag(flag, True)
cfg = DictConfig(data, parent=parent)
assert cfg == data


@mark.parametrize(
"node",
[
param(AnyNode("hello"), id="any"),
param(DictConfig({}), id="dict"),
param(ListConfig([]), id="list"),
],
)
def test_node_copy_on_set(node: Any) -> None:
cfg = OmegaConf.create({})
cfg.a = node
assert cfg.__dict__["_content"]["a"] is not node
14 changes: 14 additions & 0 deletions tests/test_basic_ops_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,17 @@ def test_listconfig_creation_with_parent_flag(flag: str) -> None:
d = [1, 2, 3]
cfg = ListConfig(d, parent=parent)
assert cfg == d


@mark.parametrize(
"node",
[
param(AnyNode("hello"), id="any"),
param(DictConfig({}), id="dict"),
param(ListConfig([]), id="list"),
],
)
def test_node_copy_on_append(node: Any) -> None:
cfg = OmegaConf.create([])
cfg.append(node)
assert cfg.__dict__["_content"][0] is not node

0 comments on commit bc03bb3

Please sign in to comment.