Skip to content

Commit

Permalink
introduced warning if user is using dot.in.key to get read for compac…
Browse files Browse the repository at this point in the history
…t keys
  • Loading branch information
omry committed Apr 23, 2020
1 parent 1aed8a3 commit d0c6e61
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 15 deletions.
2 changes: 1 addition & 1 deletion omegaconf/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ def raise_(ex: Exception, cause: Exception) -> None:
if key is not None:
child_node = node._get_node(key, validate_access=False)

full_key = node._get_full_key(key=key)
full_key = node._get_full_key(key=key, disable_warning=True)

object_type = OmegaConf.get_type(node)
object_type_str = type_str(object_type)
Expand Down
4 changes: 3 additions & 1 deletion omegaconf/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,9 @@ def _format_and_raise(
assert False

@abstractmethod
def _get_full_key(self, key: Union[str, Enum, int, None]) -> str:
def _get_full_key(
self, key: Union[str, Enum, int, None], disable_warning: bool = False
) -> str:
...

def _dereference_node(
Expand Down
10 changes: 8 additions & 2 deletions omegaconf/basecontainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,9 @@ def _validate_set(self, key: Any, value: Any) -> None:
def _value(self) -> Any:
return self.__dict__["_content"]

def _get_full_key(self, key: Union[str, Enum, int, slice, None]) -> str:
def _get_full_key(
self, key: Union[str, Enum, int, slice, None], disable_warning: bool = False
) -> str:
from .listconfig import ListConfig
from .omegaconf import _select_one

Expand Down Expand Up @@ -488,7 +490,11 @@ def prepand(full_key: str, parent_type: Any, cur_type: Any, key: Any) -> str:
if key is not None and key != "":
assert isinstance(self, Container)
cur, _ = _select_one(
c=self, key=str(key), throw_on_missing=False, throw_on_type_error=False,
c=self,
key=str(key),
throw_on_missing=False,
throw_on_type_error=False,
disable_warning=disable_warning,
)
if cur is None:
cur = self
Expand Down
39 changes: 32 additions & 7 deletions omegaconf/dictconfig.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import copy
import os
import warnings
from enum import Enum
from typing import (
AbstractSet,
Expand Down Expand Up @@ -210,16 +212,22 @@ def is_typed(c: Any) -> bool:
)
raise ValidationError(msg)

def _validate_and_normalize_key(self, key: Any) -> Union[str, Enum]:
return self._s_validate_and_normalize_key(self._metadata.key_type, key)
def _validate_and_normalize_key(
self, key: Any, disable_warning: bool = False
) -> Union[str, Enum]:
return self._s_validate_and_normalize_key(
self._metadata.key_type, key, disable_warning
)

def _s_validate_and_normalize_key(
self, key_type: Any, key: Any
self, key_type: Any, key: Any, disable_warning: bool
) -> Union[str, Enum]:
if key_type is None:
for t in (str, Enum):
try:
return self._s_validate_and_normalize_key(key_type=t, key=key)
return self._s_validate_and_normalize_key(
key_type=t, key=key, disable_warning=disable_warning
)
except KeyValidationError:
pass
raise KeyValidationError("Incompatible key type '$KEY_TYPE'")
Expand All @@ -228,6 +236,20 @@ def _s_validate_and_normalize_key(
raise KeyValidationError(
f"Key $KEY ($KEY_TYPE) is incompatible with ({key_type.__name__})"
)

if "." in key:
env_disabled_warning = (
"OC_DISABLE_DOT_ACCESS_WARNING" in os.environ
and os.environ["OC_DISABLE_DOT_ACCESS_WARNING"] == "1"
)
msg = (
f"Key with dot ({key}) are deprecated and will have different semantic meaning the next major version of OmegaConf (2.1)\n"
"See the compact keys issue for more details: https://github.com/omry/omegaconf/issues/152\n"
"You can disable this warning by setting the environment variable OC_DISABLE_DOT_ACCESS_WARNING=1"
)
if not (disable_warning or env_disabled_warning):
warnings.warn(message=msg, category=PendingDeprecationWarning)

return key
elif issubclass(key_type, Enum):
try:
Expand Down Expand Up @@ -359,10 +381,13 @@ def _get_impl(self, key: Union[str, Enum], default_value: Any) -> Any:
)

def _get_node(
self, key: Union[str, Enum], validate_access: bool = True,
self,
key: Union[str, Enum],
validate_access: bool = True,
disable_warning: bool = False,
) -> Optional[Node]:
try:
key = self._validate_and_normalize_key(key)
key = self._validate_and_normalize_key(key, disable_warning)
except KeyValidationError:
if validate_access:
raise
Expand Down Expand Up @@ -397,7 +422,7 @@ def pop(self, key: Union[str, Enum], default: Any = DEFAULT_VALUE_MARKER) -> Any
if default is not DEFAULT_VALUE_MARKER:
return default
else:
full = self._get_full_key(key)
full = self._get_full_key(key=key, disable_warning=True)
if full != key:
raise ConfigKeyError(f"Key not found: '{key}' (path: '{full}')")
else:
Expand Down
8 changes: 6 additions & 2 deletions omegaconf/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,19 @@ def _is_missing(self) -> bool:
def _is_interpolation(self) -> bool:
return _is_interpolation(self._value())

def _get_full_key(self, key: Union[str, Enum, int, None]) -> str:
def _get_full_key(
self, key: Union[str, Enum, int, None], disable_warning: bool = False
) -> str:
parent = self._get_parent()
if parent is None:
if self._metadata.key is None:
return ""
else:
return str(self._metadata.key)
else:
return parent._get_full_key(self._metadata.key)
return parent._get_full_key(
self._metadata.key, disable_warning=disable_warning
)


class AnyNode(ValueNode):
Expand Down
10 changes: 8 additions & 2 deletions omegaconf/omegaconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,11 @@ def _maybe_wrap(


def _select_one(
c: Container, key: str, throw_on_missing: bool, throw_on_type_error: bool = True
c: Container,
key: str,
throw_on_missing: bool,
throw_on_type_error: bool = True,
disable_warning: bool = False,
) -> Tuple[Optional[Node], Union[str, int]]:
from .dictconfig import DictConfig
from .listconfig import ListConfig
Expand All @@ -692,7 +696,9 @@ def _select_one(
assert isinstance(c, (DictConfig, ListConfig)), f"Unexpected type : {c}"
if isinstance(c, DictConfig):
assert isinstance(ret_key, str)
val: Optional[Node] = c._get_node(ret_key, validate_access=False)
val: Optional[Node] = c._get_node(
ret_key, validate_access=False, disable_warning=disable_warning
)
if val is not None:
if val._is_missing():
if throw_on_missing:
Expand Down
24 changes: 24 additions & 0 deletions tests/test_basic_ops_dict.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
import os
import re
import tempfile
from typing import Any, Dict, List, Optional, Union
Expand Down Expand Up @@ -89,6 +90,29 @@ def test_subscript_set() -> None:
assert {"a": "b"} == c


def test_subscript_setitem_with_dot_warning() -> None:
c = OmegaConf.create()
msg = (
"Key with dot (foo.bar) are deprecated and will have different semantic meaning the next major version of OmegaConf (2.1)\n"
"See the compact keys issue for more details: https://github.com/omry/omegaconf/issues/152\n"
"You can disable this warning by setting the environment variable OC_DISABLE_DOT_ACCESS_WARNING=1"
)

with pytest.warns(expected_warning=PendingDeprecationWarning, match=re.escape(msg)):
c.__setitem__("foo.bar", 42)

with pytest.warns(expected_warning=PendingDeprecationWarning, match=re.escape(msg)):
assert c.__getitem__("foo.bar") == 42


def test_subscript_set_with_dot_warning_suppressed(recwarn: Any, mocker: Any) -> None:
c = OmegaConf.create()
mocker.patch.dict(os.environ, {"OC_DISABLE_DOT_ACCESS_WARNING": "1"})
c.__setitem__("foo.bar", 42)
assert c.__getitem__("foo.bar") == 42
assert len(recwarn) == 0


def test_pretty_dict() -> None:
c = OmegaConf.create(dict(hello="world", list=[1, 2]))
expected = """hello: world
Expand Down

0 comments on commit d0c6e61

Please sign in to comment.