Repository: omry/omegaconf

Issue:
Iterating on DictConfig with items returns raw values that does not get resolved.
`family, name = next(iter(default.items()))`

Discussion:

Patch:
diff --git a/omegaconf/config.py b/omegaconf/config.py
index 25373d8..81461ff 100644
--- a/omegaconf/config.py
+++ b/omegaconf/config.py
@@ -39,10 +39,8 @@ def get_yaml_loader():
 class Config(object):
 
     def __init__(self):
-        """
-        Can't be instantiated
-        """
-        raise NotImplementedError
+        if type(self) == Config:
+            raise NotImplementedError
 
     def save(self, f):
         data = self.pretty()
@@ -138,7 +136,7 @@ class Config(object):
                     full_key = "{}".format(key)
                 else:
                     for parent_key, v in parent.items():
-                        if v == child:
+                        if id(v) == id(child):
                             if isinstance(child, ListConfig):
                                 full_key = "{}{}".format(parent_key, full_key)
                             else:
@@ -149,7 +147,7 @@ class Config(object):
                     full_key = "[{}]".format(key)
                 else:
                     for idx, v in enumerate(parent):
-                        if v == child:
+                        if id(v) == id(child):
                             if isinstance(child, ListConfig):
                                 full_key = "[{}]{}".format(idx, full_key)
                             else:
@@ -288,12 +286,10 @@ class Config(object):
         assert isinstance(conf, Config)
         if isinstance(conf, DictConfig):
             ret = {}
-            for key, value in conf.items():
+            for key, value in conf.items(resolve=resolve):
                 if isinstance(value, Config):
                     ret[key] = Config._to_content(value, resolve)
                 else:
-                    if resolve:
-                        value = conf[key]
                     ret[key] = value
             return ret
         elif isinstance(conf, ListConfig):
@@ -365,7 +361,7 @@ class Config(object):
             # update parents of first level Config nodes to self
             assert isinstance(node, (DictConfig, ListConfig))
             if isinstance(node, DictConfig):
-                for _key, value in node.items():
+                for _key, value in node.items(resolve=False):
                     if isinstance(value, Config):
                         value._set_parent(node)
                         re_parent(value)
@@ -445,11 +441,20 @@ class Config(object):
         return isinstance(value, tuple(valid))
 
     @staticmethod
-    def _item_eq(v1, v2):
+    def _item_eq(c1, k1, c2, k2):
+        v1 = c1.content[k1]
+        v2 = c2.content[k2]
         if isinstance(v1, BaseNode):
             v1 = v1.value()
+            if isinstance(v1, str):
+                # noinspection PyProtectedMember
+                v1 = c1._resolve_single(v1)
         if isinstance(v2, BaseNode):
             v2 = v2.value()
+            if isinstance(v2, str):
+                # noinspection PyProtectedMember
+                v2 = c2._resolve_single(v2)
+
         if isinstance(v1, Config) and isinstance(v2, Config):
             if not Config._config_eq(v1, v2):
                 return False
@@ -457,22 +462,22 @@ class Config(object):
 
     @staticmethod
     def _list_eq(l1, l2):
-        assert isinstance(l1, list)
-        assert isinstance(l2, list)
+        from .listconfig import ListConfig
+        assert isinstance(l1, ListConfig)
+        assert isinstance(l2, ListConfig)
         if len(l1) != len(l2):
             return False
         for i in range(len(l1)):
-            v1 = l1[i]
-            v2 = l2[i]
-            if not Config._item_eq(v1, v2):
+            if not Config._item_eq(l1, i, l2, i):
                 return False
 
         return True
 
     @staticmethod
-    def _dict_eq(d1, d2):
-        assert isinstance(d1, dict)
-        assert isinstance(d2, dict)
+    def _dict_conf_eq(d1, d2):
+        from .dictconfig import DictConfig
+        assert isinstance(d1, DictConfig)
+        assert isinstance(d2, DictConfig)
         if len(d1) != len(d2):
             return False
         k1 = sorted(d1.keys())
@@ -480,9 +485,7 @@ class Config(object):
         if k1 != k2:
             return False
         for k in k1:
-            v1 = d1[k]
-            v2 = d2[k]
-            if not Config._item_eq(v1, v2):
+            if not Config._item_eq(d1, k, d2, k):
                 return False
 
         return True
@@ -494,8 +497,8 @@ class Config(object):
         assert isinstance(c1, Config)
         assert isinstance(c2, Config)
         if isinstance(c1, DictConfig) and isinstance(c2, DictConfig):
-            return Config._dict_eq(c1.content, c2.content)
+            return DictConfig._dict_conf_eq(c1, c2)
         if isinstance(c1, ListConfig) and isinstance(c2, ListConfig):
-            return Config._list_eq(c1.content, c2.content)
+            return Config._list_eq(c1, c2)
         # if type does not match objects are different
         return False
diff --git a/omegaconf/dictconfig.py b/omegaconf/dictconfig.py
index a9ad915..cbf0fff 100644
--- a/omegaconf/dictconfig.py
+++ b/omegaconf/dictconfig.py
@@ -7,6 +7,7 @@ import copy
 class DictConfig(Config):
 
     def __init__(self, content, parent=None):
+        super(DictConfig, self).__init__()
         assert isinstance(content, dict)
         self.__dict__['frozen_flag'] = None
         self.__dict__['content'] = {}
@@ -84,7 +85,7 @@ class DictConfig(Config):
     def __iter__(self):
         return iter(self.keys())
 
-    def items(self):
+    def items(self, resolve=True):
         class MyItems(object):
             def __init__(self, m):
                 self.map = m
@@ -99,9 +100,12 @@ class DictConfig(Config):
 
             def next(self):
                 k = next(self.iterator)
-                v = self.map.content[k]
-                if isinstance(v, BaseNode):
-                    v = v.value()
+                if resolve:
+                    v = self.map.get(k)
+                else:
+                    v = self.map.content[k]
+                    if isinstance(v, BaseNode):
+                        v = v.value()
                 kv = (k, v)
                 return kv
 
@@ -109,9 +113,9 @@ class DictConfig(Config):
 
     def __eq__(self, other):
         if isinstance(other, dict):
-            return Config._dict_eq(self.content, other)
+            return Config._dict_conf_eq(self, DictConfig(other))
         if isinstance(other, DictConfig):
-            return Config._dict_eq(self.content, other.content)
+            return Config._dict_conf_eq(self, other)
         return NotImplemented
 
     def __ne__(self, other):
diff --git a/omegaconf/listconfig.py b/omegaconf/listconfig.py
index 3352fc4..c274f18 100644
--- a/omegaconf/listconfig.py
+++ b/omegaconf/listconfig.py
@@ -10,6 +10,7 @@ from .nodes import BaseNode, UntypedNode
 
 class ListConfig(Config):
     def __init__(self, content, parent=None):
+        super(ListConfig, self).__init__()
         assert isinstance(content, (list, tuple))
         self.__dict__['frozen_flag'] = None
         self.__dict__['content'] = []
@@ -122,9 +123,9 @@ class ListConfig(Config):
 
     def __eq__(self, other):
         if isinstance(other, list):
-            return Config._list_eq(self.content, other)
+            return Config._list_eq(self, ListConfig(other))
         if isinstance(other, ListConfig):
-            return Config._list_eq(self.content, other.content)
+            return Config._list_eq(self, other)
         return NotImplemented
 
     def __ne__(self, other):


Test patch:
diff --git a/tests/test_basic_ops_dict.py b/tests/test_basic_ops_dict.py
index b90985b..b08f3ce 100644
--- a/tests/test_basic_ops_dict.py
+++ b/tests/test_basic_ops_dict.py
@@ -6,7 +6,7 @@ import pytest
 
 from omegaconf import OmegaConf, DictConfig, Config
 from omegaconf import nodes
-from omegaconf.errors import MissingMandatoryValue, FrozenConfigError
+from omegaconf.errors import MissingMandatoryValue
 from . import IllegalType
 
 
@@ -144,8 +144,27 @@ def test_map_expansion():
 
 
 def test_items():
-    c = OmegaConf.create('{a: 2, b: 10}')
-    assert {'a': 2, 'b': 10}.items() == c.items()
+    c = OmegaConf.create(dict(a=2, b=10))
+    assert sorted([('a', 2), ('b', 10)]) == sorted(list(c.items()))
+
+
+def test_items2():
+    c = OmegaConf.create(dict(a=dict(v=1), b=dict(v=1)))
+    for k, v in c.items():
+        v.v = 2
+
+    assert c.a.v == 2
+    assert c.b.v == 2
+
+
+def test_items_with_interpolation():
+    c = OmegaConf.create(
+        dict(
+            a=2,
+            b='${a}'
+        )
+    )
+    assert list({'a': 2, 'b': 2}.items()) == list(c.items())
 
 
 def test_dict_keys():
@@ -200,15 +219,6 @@ def test_iterate_dictionary():
     assert m2 == c
 
 
-def test_items():
-    c = OmegaConf.create(dict(a=dict(v=1), b=dict(v=1)))
-    for k, v in c.items():
-        v.v = 2
-
-    assert c.a.v == 2
-    assert c.b.v == 2
-
-
 def test_dict_pop():
     c = OmegaConf.create(dict(a=1, b=2))
     assert c.pop('a') == 1
@@ -390,6 +400,22 @@ def test_dict_eq(input1, input2):
     eq(c2, input2)
 
 
+@pytest.mark.parametrize('input1, input2', [
+    (dict(a=12, b='${a}'), dict(a=12, b=12)),
+])
+def test_dict_eq_with_interpolation(input1, input2):
+    c1 = OmegaConf.create(input1)
+    c2 = OmegaConf.create(input2)
+
+    def eq(a, b):
+        assert a == b
+        assert b == a
+        assert not a != b
+        assert not b != a
+
+    eq(c1, c2)
+
+
 @pytest.mark.parametrize('input1, input2', [
     (dict(), dict(a=10)),
     ({}, []),
@@ -424,7 +450,6 @@ def test_dict_not_eq_with_another_class():
     assert OmegaConf.create() != "string"
 
 
-
 def test_hash():
     c1 = OmegaConf.create(dict(a=10))
     c2 = OmegaConf.create(dict(a=10))
diff --git a/tests/test_basic_ops_list.py b/tests/test_basic_ops_list.py
index 0afeef0..930698c 100644
--- a/tests/test_basic_ops_list.py
+++ b/tests/test_basic_ops_list.py
@@ -62,6 +62,15 @@ def test_iterate_list():
     assert items[1] == 2
 
 
+def test_items_with_interpolation():
+    c = OmegaConf.create([
+        'foo',
+        '${0}'
+    ])
+
+    assert c == ['foo', 'foo']
+
+
 def test_list_pop():
     c = OmegaConf.create([1, 2, 3, 4])
     assert c.pop(0) == 1
@@ -267,6 +276,22 @@ def test_list_eq(l1, l2):
     eq(c2, l2)
 
 
+@pytest.mark.parametrize('l1,l2', [
+    ([10, '${0}'], [10, 10])
+])
+def test_list_eq_with_interpolation(l1, l2):
+    c1 = OmegaConf.create(l1)
+    c2 = OmegaConf.create(l2)
+
+    def eq(a, b):
+        assert a == b
+        assert b == a
+        assert not a != b
+        assert not b != a
+
+    eq(c1, c2)
+
+
 @pytest.mark.parametrize('input1, input2', [
     ([], [10]),
     ([10], [11]),