Struct mode : del and pop no longer supported

omegaconf/base.py

@@ -55,13 +55,14 @@ def _get_parent(self) -> Optional["Container"]:
         assert parent is None or isinstance(parent, Container)
         return parent
 
-    def _set_flag(self, flag: str, value: Optional[bool]) -> None:
+    def _set_flag(self, flag: str, value: Optional[bool]) -> "Node":
         assert value is None or isinstance(value, bool)
         if value is None:
             if flag in self._metadata.flags:
                 del self._metadata.flags[flag]
         else:
             self._metadata.flags[flag] = value
+        return self
 
     def _get_node_flag(self, flag: str) -> Optional[bool]:
         """

omegaconf/dictconfig.py

@@ -31,6 +31,7 @@
 from .errors import (
     ConfigAttributeError,
     ConfigKeyError,
+    ConfigTypeError,
     KeyValidationError,
     MissingMandatoryValue,
     OmegaConfBaseException,
@@ -101,11 +102,14 @@ def __copy__(self) -> "DictConfig":
     def copy(self) -> "DictConfig":
         return copy.copy(self)
 
-    def _validate_get(self, key: Any, value: Any = None) -> None:
-        is_typed = self._metadata.object_type not in (Any, None,) and not is_dict(
+    def _is_typed(self) -> bool:
+        return self._metadata.object_type not in (Any, None) and not is_dict(
             self._metadata.object_type
         )
 
+    def _validate_get(self, key: Any, value: Any = None) -> None:
+        is_typed = self._is_typed()
+
         is_struct = self._get_flag("struct") is True
         if key not in self.__dict__["_content"]:
             if is_typed:
@@ -319,9 +323,26 @@ def __delitem__(self, key: Union[str, int, Enum]) -> None:
                 key=key,
                 value=None,
                 cause=ReadonlyConfigError(
-                    "Cannot delete item from read-only DictConfig"
+                    "DictConfig in read-only mode does not support deletion"
+                ),
+            )
+        if self._get_flag("struct"):
+            self._format_and_raise(
+                key=key,
+                value=None,
+                cause=ConfigTypeError(
+                    "DictConfig in struct mode does not support deletion"
                 ),
             )
+        if self._is_typed():
+            self._format_and_raise(
+                key=key,
+                value=None,
+                cause=ConfigTypeError(
+                    f"{type_str(self._metadata.object_type)} (DictConfig) does not support deletion"
+                ),
+            )
+
         del self.__dict__["_content"][key]
 
     def get(
@@ -362,15 +383,21 @@ def _get_node(
 
     def pop(self, key: Union[str, Enum], default: Any = DEFAULT_VALUE_MARKER) -> Any:
         try:
-            key = self._validate_and_normalize_key(key)
             if self._get_flag("readonly"):
                 raise ReadonlyConfigError("Cannot pop from read-only node")
-
+            if self._get_flag("struct"):
+                raise ConfigTypeError("DictConfig in struct mode does not support pop")
+            if self._is_typed():
+                raise ConfigTypeError(
+                    f"{type_str(self._metadata.object_type)} (DictConfig) does not support pop"
+                )
+            key = self._validate_and_normalize_key(key)
             node = self._get_node(key=key, validate_access=False)
             if node is not None:
                 value = self._resolve_with_default(
                     key=key, value=node, default_value=default
                 )
+
                 del self[key]
                 return value
             else:

tests/test_basic_ops_dict.py

@@ -15,7 +15,7 @@
     _utils,
 )
 from omegaconf.basecontainer import BaseContainer
-from omegaconf.errors import KeyValidationError
+from omegaconf.errors import ConfigTypeError, KeyValidationError
 
 from . import (
     ConcretePlugin,
@@ -255,14 +255,6 @@ def test_iterate_dict_with_interpolation() -> None:
         i = i + 1
 
 
-@pytest.mark.parametrize(  # type: ignore
-    "struct",
-    [
-        pytest.param(None, id="struct_unspecified"),
-        pytest.param(False, id="struct_False"),
-        pytest.param(True, id="struct_True"),
-    ],
-)
 @pytest.mark.parametrize(  # type: ignore
     "cfg, key, default_, expected",
     [
@@ -298,11 +290,8 @@ def test_iterate_dict_with_interpolation() -> None:
         ),
     ],
 )
-def test_dict_pop(
-    cfg: Dict[Any, Any], key: Any, default_: Any, expected: Any, struct: Optional[bool]
-) -> None:
+def test_dict_pop(cfg: Dict[Any, Any], key: Any, default_: Any, expected: Any) -> None:
     c = OmegaConf.create(cfg)
-    OmegaConf.set_struct(c, struct)
 
     if default_ != "__NO_DEFAULT__":
         val = c.pop(key, default_)
@@ -313,6 +302,24 @@ def test_dict_pop(
     assert type(val) == type(expected)
 
 
+@pytest.mark.parametrize(  # type: ignore
+    "cfg",
+    [
+        (OmegaConf.create({"name": "Bond", "age": 7})._set_flag("struct", True)),
+        (OmegaConf.structured(User)),
+    ],
+)
+def test_dict_struct_mode_pop(cfg: Any) -> None:
+    with pytest.raises(ConfigTypeError):
+        cfg.pop("name")
+
+    with pytest.raises(ConfigTypeError):
+        cfg.pop("bar")
+
+    with pytest.raises(ConfigTypeError):
+        cfg.pop("bar", "not even with default")
+
+
 @pytest.mark.parametrize(  # type: ignore
     "cfg, key, expectation",
     [
@@ -377,14 +384,23 @@ def test_dict_config() -> None:
 
 
 def test_dict_delitem() -> None:
-    c = OmegaConf.create(dict(a=10, b=11))
-    assert c == dict(a=10, b=11)
+    src = {"a": 10, "b": 11}
+    c = OmegaConf.create(src)
+    assert c == src
     del c["a"]
-    assert c == dict(b=11)
+    assert c == {"b": 11}
     with pytest.raises(KeyError):
         del c["not_found"]
 
 
+def test_dict_struct_delitem() -> None:
+    src = {"a": 10, "b": 11}
+    c = OmegaConf.create(src)
+    OmegaConf.set_struct(c, True)
+    with pytest.raises(ConfigTypeError):
+        del c["a"]
+
+
 @pytest.mark.parametrize(  # type: ignore
     "d, expected", [({}, 0), ({"a": 10, "b": 11}, 2)],
 )

tests/test_errors.py

@@ -31,6 +31,7 @@
     Plugin,
     StructuredWithMissing,
     UnionError,
+    User,
 )
 
 
@@ -449,12 +450,35 @@ def finalize(self, cfg: Any) -> None:
             create=lambda: create_readonly({"foo": "bar"}),
             op=lambda cfg: cfg.__delitem__("foo"),
             exception_type=ReadonlyConfigError,
-            msg="Cannot delete item from read-only DictConfig",
+            msg="DictConfig in read-only mode does not support deletion",
             key="foo",
             child_node=lambda cfg: cfg.foo,
         ),
         id="dict,readonly:del",
     ),
+    pytest.param(
+        Expected(
+            create=lambda: create_struct({"foo": "bar"}),
+            op=lambda cfg: cfg.__delitem__("foo"),
+            exception_type=ConfigTypeError,
+            msg="DictConfig in struct mode does not support deletion",
+            key="foo",
+            child_node=lambda cfg: cfg.foo,
+        ),
+        id="dict,struct:del",
+    ),
+    pytest.param(
+        Expected(
+            create=lambda: OmegaConf.structured(User(name="bond")),
+            op=lambda cfg: cfg.__delitem__("name"),
+            exception_type=ConfigTypeError,
+            msg="User (DictConfig) does not support deletion",
+            object_type=User,
+            key="name",
+            child_node=lambda cfg: cfg.name,
+        ),
+        id="dict,structured:del",
+    ),
     ##############
     # ListConfig #
     ##############
@@ -578,6 +602,28 @@ def finalize(self, cfg: Any) -> None:
         ),
         id="list:pop_invalid_key",
     ),
+    pytest.param(
+        Expected(
+            create=lambda: create_struct({"foo": "bar"}),
+            op=lambda cfg: cfg.pop("foo"),
+            exception_type=ConfigTypeError,
+            msg="DictConfig in struct mode does not support pop",
+            key="foo",
+            child_node=lambda cfg: cfg.foo,
+        ),
+        id="dict,struct:pop",
+    ),
+    pytest.param(
+        Expected(
+            create=lambda: OmegaConf.structured(ConcretePlugin),
+            op=lambda cfg: cfg.pop("name"),
+            exception_type=ConfigTypeError,
+            msg="ConcretePlugin (DictConfig) does not support pop",
+            key="name",
+            child_node=lambda cfg: cfg.name,
+        ),
+        id="dict,structured:pop",
+    ),
     pytest.param(
         Expected(
             create=lambda: ListConfig(content=None),