From f397888dacd7d3ee572bcd8466e1f5a81b13c1d3 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Wed, 5 May 2021 22:38:56 -0500 Subject: [PATCH 01/13] update docs for subclassing Dict --- docs/source/structured_config.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/source/structured_config.rst b/docs/source/structured_config.rst index b0c05524e..e0d4e43e8 100644 --- a/docs/source/structured_config.rst +++ b/docs/source/structured_config.rst @@ -99,6 +99,8 @@ You can create a config with specified fields that can also accept arbitrary val >>> conf.foo = "bar" >>> assert conf.foo == "bar" +This feature is deprecated; OmegaConf's ability to handle structured configs +that subclass ``Dict`` is planned to be removed in a future release. Static type checker support From 54c1c655aff76183cd3ce2eb05271670bf5c561b Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Thu, 6 May 2021 02:13:19 -0500 Subject: [PATCH 02/13] issue deprecation warning on Dict subclass construction --- omegaconf/_utils.py | 10 ++++- .../structured_conf/test_structured_config.py | 43 ++++++++++++++----- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index c87bedb81..5db9e5b3c 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -3,6 +3,7 @@ import re import string import sys +import warnings from contextlib import contextmanager from enum import Enum from textwrap import dedent @@ -234,10 +235,17 @@ def extract_dict_subclass_data(obj: Any, parent: Any) -> Optional[Dict[str, Any] """Check if obj is an instance of a subclass of Dict. If so, extract the Dict keys/values.""" from omegaconf.omegaconf import _maybe_wrap + obj_type = type(obj) + if is_dict_subclass(obj) or is_dict_subclass(obj_type): + warnings.warn( + "Subclassing of `Dict` by Structured Config classes is deprecated", + UserWarning, + stacklevel=2, + ) + if isinstance(obj, type): return None - obj_type = type(obj) if is_dict_subclass(obj_type): dict_subclass_data = {} key_type, element_type = get_dict_key_value_types(obj_type) diff --git a/tests/structured_conf/test_structured_config.py b/tests/structured_conf/test_structured_config.py index c6578f3e8..d93cb926c 100644 --- a/tests/structured_conf/test_structured_config.py +++ b/tests/structured_conf/test_structured_config.py @@ -1,8 +1,9 @@ +import re import sys from importlib import import_module from typing import Any, Dict, List, Optional -from pytest import fixture, mark, param, raises +from pytest import fixture, mark, param, raises, warns from omegaconf import ( MISSING, @@ -932,8 +933,17 @@ def test_dataclass_frozen() -> None: class TestDictSubclass: + def warns_deprecated(self) -> Any: + return warns( + UserWarning, + match=re.escape( + "Subclassing of `Dict` by Structured Config classes is deprecated" + ), + ) + def test_str2str(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2Str()) + with self.warns_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Str2Str()) cfg.hello = "world" assert cfg.hello == "world" @@ -943,7 +953,8 @@ def test_str2str(self, module: Any) -> None: def test_dict_subclass_data_preserved_upon_node_creation(self, module: Any) -> None: src = module.DictSubclass.Str2StrWithField() src["baz"] = "qux" - cfg = OmegaConf.structured(src) + with self.warns_deprecated(): + cfg = OmegaConf.structured(src) assert cfg.foo == "bar" assert cfg.baz == "qux" @@ -954,7 +965,8 @@ def test_create_dict_subclass_with_bad_value_type(self, module: Any) -> None: OmegaConf.structured(src) def test_str2str_as_sub_node(self, module: Any) -> None: - cfg = OmegaConf.create({"foo": module.DictSubclass.Str2Str}) + with self.warns_deprecated(): + cfg = OmegaConf.create({"foo": module.DictSubclass.Str2Str}) assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Str2Str assert _utils.get_ref_type(cfg.foo) == Any @@ -968,7 +980,8 @@ def test_str2str_as_sub_node(self, module: Any) -> None: cfg.foo[123] = "fail" def test_int2str(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Int2Str()) + with self.warns_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Int2Str()) cfg[10] = "ten" # okay assert cfg[10] == "ten" @@ -986,7 +999,8 @@ def test_int2str(self, module: Any) -> None: cfg[Color.RED] = "fail" def test_int2str_as_sub_node(self, module: Any) -> None: - cfg = OmegaConf.create({"foo": module.DictSubclass.Int2Str}) + with self.warns_deprecated(): + cfg = OmegaConf.create({"foo": module.DictSubclass.Int2Str}) assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Int2Str assert _utils.get_ref_type(cfg.foo) == Any @@ -1006,7 +1020,8 @@ def test_int2str_as_sub_node(self, module: Any) -> None: cfg.foo[Color.RED] = "fail" def test_color2str(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Color2Str()) + with self.warns_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Color2Str()) cfg[Color.RED] = "red" with raises(KeyValidationError): @@ -1016,7 +1031,8 @@ def test_color2str(self, module: Any) -> None: cfg[123] = "nope" def test_color2color(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Color2Color()) + with self.warns_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Color2Color()) # add key cfg[Color.RED] = "GREEN" @@ -1045,7 +1061,8 @@ def test_color2color(self, module: Any) -> None: cfg.greeen = "nope" def test_str2user(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2User()) + with self.warns_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Str2User()) cfg.bond = module.User(name="James Bond", age=7) assert cfg.bond.name == "James Bond" @@ -1060,7 +1077,8 @@ def test_str2user(self, module: Any) -> None: cfg[Color.BLUE] = "nope" def test_str2str_with_field(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) + with self.warns_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) assert cfg.foo == "bar" cfg.hello = "world" assert cfg.hello == "world" @@ -1074,10 +1092,13 @@ def test_usr2str(self, module: Any) -> None: OmegaConf.structured(module.DictSubclass.Error.User2Str()) def test_str2int_with_field_of_different_type(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2IntWithStrField()) + with warns(UserWarning): + cfg = OmegaConf.structured(module.DictSubclass.Str2IntWithStrField()) with raises(ValidationError): cfg.foo = "str" + +class TestConfigs2: def test_construct_from_another_retain_node_types(self, module: Any) -> None: cfg1 = OmegaConf.create(module.User(name="James Bond", age=7)) with raises(ValidationError): From ae6cb72472d38bfb39aaa4b98a2358d60f71b051 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Fri, 7 May 2021 14:35:31 -0500 Subject: [PATCH 03/13] remove documentation for extending Dict --- docs/source/structured_config.rst | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/docs/source/structured_config.rst b/docs/source/structured_config.rst index e0d4e43e8..1f2ee5a54 100644 --- a/docs/source/structured_config.rst +++ b/docs/source/structured_config.rst @@ -85,23 +85,6 @@ The resulting object and will also rejects attempts to access or set fields that >>> with raises(AttributeError): ... conf.does_not_exist -You can create a config with specified fields that can also accept arbitrary values by extending Dict: - -.. doctest:: - - >>> @dataclass - ... class DictWithFields(Dict[str, Any]): - ... num: int = 10 - >>> - >>> conf = OmegaConf.structured(DictWithFields) - >>> assert conf.num == 10 - >>> - >>> conf.foo = "bar" - >>> assert conf.foo == "bar" - -This feature is deprecated; OmegaConf's ability to handle structured configs -that subclass ``Dict`` is planned to be removed in a future release. - Static type checker support ^^^^^^^^^^^^^^^^^^^^^^^^^^^ From c6054dbdcdfdcc3e7cdeef9cab6e56c2057574b7 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Fri, 7 May 2021 14:41:15 -0500 Subject: [PATCH 04/13] make warns_dict_subclass_deprecated global-scoped --- .../structured_conf/test_structured_config.py | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/tests/structured_conf/test_structured_config.py b/tests/structured_conf/test_structured_config.py index d93cb926c..112fadeb6 100644 --- a/tests/structured_conf/test_structured_config.py +++ b/tests/structured_conf/test_structured_config.py @@ -932,17 +932,18 @@ def test_dataclass_frozen() -> None: validate_frozen_impl(OmegaConf.structured(FrozenClass())) -class TestDictSubclass: - def warns_deprecated(self) -> Any: - return warns( - UserWarning, - match=re.escape( - "Subclassing of `Dict` by Structured Config classes is deprecated" - ), - ) +def warns_dict_subclass_deprecated() -> Any: + return warns( + UserWarning, + match=re.escape( + "Subclassing of `Dict` by Structured Config classes is deprecated" + ), + ) + +class TestDictSubclass: def test_str2str(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Str2Str()) cfg.hello = "world" assert cfg.hello == "world" @@ -953,7 +954,7 @@ def test_str2str(self, module: Any) -> None: def test_dict_subclass_data_preserved_upon_node_creation(self, module: Any) -> None: src = module.DictSubclass.Str2StrWithField() src["baz"] = "qux" - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(src) assert cfg.foo == "bar" assert cfg.baz == "qux" @@ -965,7 +966,7 @@ def test_create_dict_subclass_with_bad_value_type(self, module: Any) -> None: OmegaConf.structured(src) def test_str2str_as_sub_node(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.create({"foo": module.DictSubclass.Str2Str}) assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Str2Str assert _utils.get_ref_type(cfg.foo) == Any @@ -980,7 +981,7 @@ def test_str2str_as_sub_node(self, module: Any) -> None: cfg.foo[123] = "fail" def test_int2str(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Int2Str()) cfg[10] = "ten" # okay @@ -999,7 +1000,7 @@ def test_int2str(self, module: Any) -> None: cfg[Color.RED] = "fail" def test_int2str_as_sub_node(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.create({"foo": module.DictSubclass.Int2Str}) assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Int2Str assert _utils.get_ref_type(cfg.foo) == Any @@ -1020,7 +1021,7 @@ def test_int2str_as_sub_node(self, module: Any) -> None: cfg.foo[Color.RED] = "fail" def test_color2str(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Color2Str()) cfg[Color.RED] = "red" @@ -1031,7 +1032,7 @@ def test_color2str(self, module: Any) -> None: cfg[123] = "nope" def test_color2color(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Color2Color()) # add key @@ -1061,7 +1062,7 @@ def test_color2color(self, module: Any) -> None: cfg.greeen = "nope" def test_str2user(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Str2User()) cfg.bond = module.User(name="James Bond", age=7) @@ -1077,7 +1078,7 @@ def test_str2user(self, module: Any) -> None: cfg[Color.BLUE] = "nope" def test_str2str_with_field(self, module: Any) -> None: - with self.warns_deprecated(): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) assert cfg.foo == "bar" cfg.hello = "world" @@ -1092,7 +1093,7 @@ def test_usr2str(self, module: Any) -> None: OmegaConf.structured(module.DictSubclass.Error.User2Str()) def test_str2int_with_field_of_different_type(self, module: Any) -> None: - with warns(UserWarning): + with warns_dict_subclass_deprecated(): cfg = OmegaConf.structured(module.DictSubclass.Str2IntWithStrField()) with raises(ValidationError): cfg.foo = "str" From 61fda908fc27bb20f217e50008e3e56188c21aac Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Fri, 7 May 2021 15:55:37 -0500 Subject: [PATCH 05/13] catch additional warnings --- tests/__init__.py | 11 +++++++++++ tests/structured_conf/test_structured_config.py | 17 ++++------------- tests/test_errors.py | 14 -------------- tests/test_to_container.py | 11 +++++++---- 4 files changed, 22 insertions(+), 31 deletions(-) diff --git a/tests/__init__.py b/tests/__init__.py index be1abf765..16a28f529 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,8 +1,10 @@ +import re from dataclasses import dataclass, field from enum import Enum from typing import Any, Dict, List, Optional, Union import attr +from pytest import warns from omegaconf import II, MISSING @@ -220,3 +222,12 @@ class InterpolationDict: @dataclass class Str2Int(Dict[str, int]): pass + + +def warns_dict_subclass_deprecated() -> Any: + return warns( + UserWarning, + match=re.escape( + "Subclassing of `Dict` by Structured Config classes is deprecated" + ), + ) diff --git a/tests/structured_conf/test_structured_config.py b/tests/structured_conf/test_structured_config.py index 112fadeb6..4c1b9a06a 100644 --- a/tests/structured_conf/test_structured_config.py +++ b/tests/structured_conf/test_structured_config.py @@ -1,9 +1,8 @@ -import re import sys from importlib import import_module from typing import Any, Dict, List, Optional -from pytest import fixture, mark, param, raises, warns +from pytest import fixture, mark, param, raises from omegaconf import ( MISSING, @@ -18,7 +17,7 @@ _utils, ) from omegaconf.errors import ConfigKeyError -from tests import Color, User +from tests import Color, User, warns_dict_subclass_deprecated @fixture( @@ -932,15 +931,6 @@ def test_dataclass_frozen() -> None: validate_frozen_impl(OmegaConf.structured(FrozenClass())) -def warns_dict_subclass_deprecated() -> Any: - return warns( - UserWarning, - match=re.escape( - "Subclassing of `Dict` by Structured Config classes is deprecated" - ), - ) - - class TestDictSubclass: def test_str2str(self, module: Any) -> None: with warns_dict_subclass_deprecated(): @@ -963,7 +953,8 @@ def test_create_dict_subclass_with_bad_value_type(self, module: Any) -> None: src = module.DictSubclass.Str2Int() src["baz"] = "qux" with raises(ValidationError): - OmegaConf.structured(src) + with warns_dict_subclass_deprecated(): + OmegaConf.structured(src) def test_str2str_as_sub_node(self, module: Any) -> None: with warns_dict_subclass_deprecated(): diff --git a/tests/test_errors.py b/tests/test_errors.py index c6abe3efa..eb24e42e6 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -38,7 +38,6 @@ Module, Package, Plugin, - Str2Int, StructuredWithBadDict, StructuredWithBadList, StructuredWithMissing, @@ -793,19 +792,6 @@ def finalize(self, cfg: Any) -> None: id="dict,readonly:delattr", ), # creating structured config - param( - Expected( - create=lambda: Str2Int(), - op=lambda src: (src.__setitem__("bar", "qux"), OmegaConf.structured(src)), - exception_type=ValidationError, - msg="Value 'qux' could not be converted to Integer", - object_type=None, - key="bar", - full_key="bar", - parent_node=lambda cfg: None, - ), - id="structured,Dict_subclass:bad_value_type", - ), param( Expected( create=lambda: None, diff --git a/tests/test_to_container.py b/tests/test_to_container.py index 4d881613e..edf65578e 100644 --- a/tests/test_to_container.py +++ b/tests/test_to_container.py @@ -14,7 +14,7 @@ open_dict, ) from omegaconf.errors import InterpolationResolutionError -from tests import Color, User +from tests import Color, User, warns_dict_subclass_deprecated @mark.parametrize( @@ -377,7 +377,8 @@ def test_nested_object_with_Any_ref_type(self, module: Any) -> None: assert nested.var.interpolation == 456 def test_str2user_instantiate(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2User()) + with warns_dict_subclass_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Str2User()) cfg.bond = module.User(name="James Bond", age=7) data = self.round_trip_to_object(cfg) @@ -386,7 +387,8 @@ def test_str2user_instantiate(self, module: Any) -> None: assert data.bond == module.User("James Bond", 7) def test_str2user_with_field_instantiate(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2UserWithField()) + with warns_dict_subclass_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Str2UserWithField()) cfg.mp = module.User(name="Moneypenny", age=11) data = self.round_trip_to_object(cfg) @@ -397,7 +399,8 @@ def test_str2user_with_field_instantiate(self, module: Any) -> None: assert data.mp == module.User("Moneypenny", 11) def test_str2str_with_field_instantiate(self, module: Any) -> None: - cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) + with warns_dict_subclass_deprecated(): + cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) cfg.hello = "world" data = self.round_trip_to_object(cfg) From 0e5d0464fc5e8d730468e8a0d45175eb3ac4a648 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Fri, 7 May 2021 21:02:35 -0500 Subject: [PATCH 06/13] change warning stacklevel 2 -> 1 --- omegaconf/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index 5db9e5b3c..237e1db12 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -240,7 +240,7 @@ def extract_dict_subclass_data(obj: Any, parent: Any) -> Optional[Dict[str, Any] warnings.warn( "Subclassing of `Dict` by Structured Config classes is deprecated", UserWarning, - stacklevel=2, + stacklevel=1, ) if isinstance(obj, type): From ea0aa29ca3c180318859d331e0219e6b7c6e73dd Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Fri, 7 May 2021 21:13:28 -0500 Subject: [PATCH 07/13] make the UserWarning point to issue #663 --- omegaconf/_utils.py | 2 +- tests/__init__.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index 237e1db12..05c75a9d1 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -238,7 +238,7 @@ def extract_dict_subclass_data(obj: Any, parent: Any) -> Optional[Dict[str, Any] obj_type = type(obj) if is_dict_subclass(obj) or is_dict_subclass(obj_type): warnings.warn( - "Subclassing of `Dict` by Structured Config classes is deprecated", + "Subclassing `Dict` in Structured Config classes is deprecated, see github.com/omry/omegaconf/issues/663", UserWarning, stacklevel=1, ) diff --git a/tests/__init__.py b/tests/__init__.py index 16a28f529..25b2600fb 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -228,6 +228,6 @@ def warns_dict_subclass_deprecated() -> Any: return warns( UserWarning, match=re.escape( - "Subclassing of `Dict` by Structured Config classes is deprecated" + "Subclassing `Dict` in Structured Config classes is deprecated, see github.com/omry/omegaconf/issues/663" ), ) From 6eeb616e432eefa086fd17d7c813521417fc32de Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 10 May 2021 16:19:09 -0500 Subject: [PATCH 08/13] stacklevel 1 -> 9 (point to the line calling OmegaConf.create) --- omegaconf/_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index 05c75a9d1..1efcfa2cc 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -240,7 +240,7 @@ def extract_dict_subclass_data(obj: Any, parent: Any) -> Optional[Dict[str, Any] warnings.warn( "Subclassing `Dict` in Structured Config classes is deprecated, see github.com/omry/omegaconf/issues/663", UserWarning, - stacklevel=1, + stacklevel=9, ) if isinstance(obj, type): From 71ad2b61c27fca5aee0dce609f06a44e5e0ac096 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 10 May 2021 16:47:56 -0500 Subject: [PATCH 09/13] clean up control flow and include class name in warning msg --- omegaconf/_utils.py | 20 +++++++++------- tests/__init__.py | 6 +++-- .../structured_conf/test_structured_config.py | 24 ++++++++++--------- tests/test_to_container.py | 6 ++--- 4 files changed, 32 insertions(+), 24 deletions(-) diff --git a/omegaconf/_utils.py b/omegaconf/_utils.py index 1efcfa2cc..d3dafe9b8 100644 --- a/omegaconf/_utils.py +++ b/omegaconf/_utils.py @@ -235,18 +235,22 @@ def extract_dict_subclass_data(obj: Any, parent: Any) -> Optional[Dict[str, Any] """Check if obj is an instance of a subclass of Dict. If so, extract the Dict keys/values.""" from omegaconf.omegaconf import _maybe_wrap - obj_type = type(obj) - if is_dict_subclass(obj) or is_dict_subclass(obj_type): + is_type = isinstance(obj, type) + obj_type = obj if is_type else type(obj) + subclasses_dict = is_dict_subclass(obj_type) + + if subclasses_dict: warnings.warn( - "Subclassing `Dict` in Structured Config classes is deprecated, see github.com/omry/omegaconf/issues/663", + f"Class `{obj_type.__name__}` subclasses `Dict`." + + " Subclassing `Dict` in Structured Config classes is deprecated," + + " see github.com/omry/omegaconf/issues/663", UserWarning, stacklevel=9, ) - if isinstance(obj, type): + if is_type: return None - - if is_dict_subclass(obj_type): + elif subclasses_dict: dict_subclass_data = {} key_type, element_type = get_dict_key_value_types(obj_type) for name, value in obj.items(): @@ -265,8 +269,8 @@ def extract_dict_subclass_data(obj: Any, parent: Any) -> Optional[Dict[str, Any] node=None, key=name, value=value, cause=ex, msg=str(ex) ) return dict_subclass_data - - return None + else: + return None def get_attr_class_field_names(obj: Any) -> List[str]: diff --git a/tests/__init__.py b/tests/__init__.py index 25b2600fb..fe574e446 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -224,10 +224,12 @@ class Str2Int(Dict[str, int]): pass -def warns_dict_subclass_deprecated() -> Any: +def warns_dict_subclass_deprecated(dict_subclass: Any) -> Any: return warns( UserWarning, match=re.escape( - "Subclassing `Dict` in Structured Config classes is deprecated, see github.com/omry/omegaconf/issues/663" + f"Class `{dict_subclass.__name__}` subclasses `Dict`." + + " Subclassing `Dict` in Structured Config classes is deprecated," + + " see github.com/omry/omegaconf/issues/663" ), ) diff --git a/tests/structured_conf/test_structured_config.py b/tests/structured_conf/test_structured_config.py index 4c1b9a06a..2d14570e6 100644 --- a/tests/structured_conf/test_structured_config.py +++ b/tests/structured_conf/test_structured_config.py @@ -933,7 +933,7 @@ def test_dataclass_frozen() -> None: class TestDictSubclass: def test_str2str(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2Str): cfg = OmegaConf.structured(module.DictSubclass.Str2Str()) cfg.hello = "world" assert cfg.hello == "world" @@ -944,7 +944,7 @@ def test_str2str(self, module: Any) -> None: def test_dict_subclass_data_preserved_upon_node_creation(self, module: Any) -> None: src = module.DictSubclass.Str2StrWithField() src["baz"] = "qux" - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2StrWithField): cfg = OmegaConf.structured(src) assert cfg.foo == "bar" assert cfg.baz == "qux" @@ -953,11 +953,11 @@ def test_create_dict_subclass_with_bad_value_type(self, module: Any) -> None: src = module.DictSubclass.Str2Int() src["baz"] = "qux" with raises(ValidationError): - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2Int): OmegaConf.structured(src) def test_str2str_as_sub_node(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2Str): cfg = OmegaConf.create({"foo": module.DictSubclass.Str2Str}) assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Str2Str assert _utils.get_ref_type(cfg.foo) == Any @@ -972,7 +972,7 @@ def test_str2str_as_sub_node(self, module: Any) -> None: cfg.foo[123] = "fail" def test_int2str(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Int2Str): cfg = OmegaConf.structured(module.DictSubclass.Int2Str()) cfg[10] = "ten" # okay @@ -991,7 +991,7 @@ def test_int2str(self, module: Any) -> None: cfg[Color.RED] = "fail" def test_int2str_as_sub_node(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Int2Str): cfg = OmegaConf.create({"foo": module.DictSubclass.Int2Str}) assert OmegaConf.get_type(cfg.foo) == module.DictSubclass.Int2Str assert _utils.get_ref_type(cfg.foo) == Any @@ -1012,7 +1012,7 @@ def test_int2str_as_sub_node(self, module: Any) -> None: cfg.foo[Color.RED] = "fail" def test_color2str(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Color2Str): cfg = OmegaConf.structured(module.DictSubclass.Color2Str()) cfg[Color.RED] = "red" @@ -1023,7 +1023,7 @@ def test_color2str(self, module: Any) -> None: cfg[123] = "nope" def test_color2color(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Color2Color): cfg = OmegaConf.structured(module.DictSubclass.Color2Color()) # add key @@ -1053,7 +1053,7 @@ def test_color2color(self, module: Any) -> None: cfg.greeen = "nope" def test_str2user(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2User): cfg = OmegaConf.structured(module.DictSubclass.Str2User()) cfg.bond = module.User(name="James Bond", age=7) @@ -1069,7 +1069,7 @@ def test_str2user(self, module: Any) -> None: cfg[Color.BLUE] = "nope" def test_str2str_with_field(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2StrWithField): cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) assert cfg.foo == "bar" cfg.hello = "world" @@ -1084,7 +1084,9 @@ def test_usr2str(self, module: Any) -> None: OmegaConf.structured(module.DictSubclass.Error.User2Str()) def test_str2int_with_field_of_different_type(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated( + module.DictSubclass.Str2IntWithStrField + ): cfg = OmegaConf.structured(module.DictSubclass.Str2IntWithStrField()) with raises(ValidationError): cfg.foo = "str" diff --git a/tests/test_to_container.py b/tests/test_to_container.py index edf65578e..36af3d66b 100644 --- a/tests/test_to_container.py +++ b/tests/test_to_container.py @@ -377,7 +377,7 @@ def test_nested_object_with_Any_ref_type(self, module: Any) -> None: assert nested.var.interpolation == 456 def test_str2user_instantiate(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2User): cfg = OmegaConf.structured(module.DictSubclass.Str2User()) cfg.bond = module.User(name="James Bond", age=7) data = self.round_trip_to_object(cfg) @@ -387,7 +387,7 @@ def test_str2user_instantiate(self, module: Any) -> None: assert data.bond == module.User("James Bond", 7) def test_str2user_with_field_instantiate(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2UserWithField): cfg = OmegaConf.structured(module.DictSubclass.Str2UserWithField()) cfg.mp = module.User(name="Moneypenny", age=11) data = self.round_trip_to_object(cfg) @@ -399,7 +399,7 @@ def test_str2user_with_field_instantiate(self, module: Any) -> None: assert data.mp == module.User("Moneypenny", 11) def test_str2str_with_field_instantiate(self, module: Any) -> None: - with warns_dict_subclass_deprecated(): + with warns_dict_subclass_deprecated(module.DictSubclass.Str2StrWithField): cfg = OmegaConf.structured(module.DictSubclass.Str2StrWithField()) cfg.hello = "world" data = self.round_trip_to_object(cfg) From 11c210ebf8a20b14d1c62ade651d3206cfa0172b Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Mon, 10 May 2021 18:26:08 -0500 Subject: [PATCH 10/13] test_errors.py:test_dict_subclass_error --- tests/test_errors.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/test_errors.py b/tests/test_errors.py index eb24e42e6..9b0a39df3 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -38,12 +38,14 @@ Module, Package, Plugin, + Str2Int, StructuredWithBadDict, StructuredWithBadList, StructuredWithMissing, SubscriptedDict, UnionError, User, + warns_dict_subclass_deprecated, ) @@ -1431,3 +1433,24 @@ def test_get_full_key_failure_in_format_and_raise() -> None: with raises(RecursionError, match=match): c.x + + +def test_dict_subclass_error() -> None: + src = Str2Int() + src["bar"] = "qux" # type: ignore + # expected.finalize(cfg) + with raises( + ValidationError, + match=re.escape("Value 'qux' could not be converted to Integer"), + ) as einfo: + with warns_dict_subclass_deprecated(Str2Int): + OmegaConf.structured(src) + ex = einfo.value + assert isinstance(ex, OmegaConfBaseException) + + assert ex.key == "bar" + assert ex.full_key == "bar" + assert ex.ref_type is None + assert ex.object_type is None + assert ex.parent_node is None + assert ex.child_node is None From b58bcd597556b056285e03b1396851ab021f231a Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Tue, 11 May 2021 17:45:08 -0500 Subject: [PATCH 11/13] add docstring to test_dict_subclass_error function --- tests/test_errors.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_errors.py b/tests/test_errors.py index 9b0a39df3..5465e5a07 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -1436,6 +1436,12 @@ def test_get_full_key_failure_in_format_and_raise() -> None: def test_dict_subclass_error() -> None: + """ + Test calling OmegaConf.structured(malformed_dict_subclass). + We expect a ValueError and a UserWarning (deprecation) to be raised simultaneously. + We are using a separate function instead of adding + warning support to the giant `test_errors` function above, + """ src = Str2Int() src["bar"] = "qux" # type: ignore # expected.finalize(cfg) From 192c9dbe72d36d3329fd9d3fd660bdfc17c1fa82 Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Tue, 11 May 2021 17:46:07 -0500 Subject: [PATCH 12/13] delete extraneous comment --- tests/test_errors.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_errors.py b/tests/test_errors.py index 5465e5a07..203ee718f 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -1444,7 +1444,6 @@ def test_dict_subclass_error() -> None: """ src = Str2Int() src["bar"] = "qux" # type: ignore - # expected.finalize(cfg) with raises( ValidationError, match=re.escape("Value 'qux' could not be converted to Integer"), From 04f8a6db3dc95ec52c727d292c412df1df67036c Mon Sep 17 00:00:00 2001 From: Jasha <8935917+Jasha10@users.noreply.github.com> Date: Tue, 11 May 2021 17:48:41 -0500 Subject: [PATCH 13/13] add news fragment --- news/663.api_change | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/663.api_change diff --git a/news/663.api_change b/news/663.api_change new file mode 100644 index 000000000..bf065c717 --- /dev/null +++ b/news/663.api_change @@ -0,0 +1 @@ +Support for Structured Configs that subclass `typing.Dict` is now deprecated.