Repository: ClimateImpactLab/impactlab-tools

Issue:
`gather_configtree()` needs option to assign parents to nested lists of dicts
`gather_configtree()` will skip over any nested lists of dictionaries. @jrising suggested allowing `gather_configtree()` to convert these nested dicts to `ConfDicts` and assigning them parents, as we would do for any other nested dictionaries.

The key here is that using the `accessed_all_keys()` method with `search='parents'` or `search='children'` from the top or bottom of the tree should not be "blocked" by a list of ConfDicts. This was a cause for trouble in the `impact-calculations` package. Be sure to test against this use case.

Discussion:

Patch:
diff --git a/impactlab_tools/utils/configdict.py b/impactlab_tools/utils/configdict.py
index cb9e5ec..135dfde 100644
--- a/impactlab_tools/utils/configdict.py
+++ b/impactlab_tools/utils/configdict.py
@@ -10,15 +10,19 @@ except ImportError:
     import collections as collections_abc
 
 
-def gather_configtree(d):
+def gather_configtree(d, parse_lists=False):
     """Chains nested-dicts into a connected tree of ConfigDict(s)
 
     Parameters
     ----------
-    d : dict
+    d : dict or MutableMapping
         Cast to :py:class:`ConfigDict`. Nested dicts within are also
         recursively cast and assigned parents, reflecting their nested
         structure.
+    parse_lists : bool, optional
+        If `d` or its children contain a list of dicts, do you want to convert
+        these listed dicts to ConfDicts and assign them parents. This is
+        slow. Note this only parses lists, strictly, not all Sequences.
 
     Returns
     -------
@@ -47,8 +51,17 @@ def gather_configtree(d):
     for k, v in out.data.items():
         # Replace nested maps with new ConfigDicts
         if isinstance(v, collections_abc.MutableMapping):
-            out.data[k] = gather_configtree(v)
+            out.data[k] = gather_configtree(v, parse_lists=parse_lists)
             out.data[k].parent = out
+
+        # If list has mappings, replace mappings with new ConfigDicts
+        if parse_lists and isinstance(v, list):
+            for idx, item in enumerate(v):
+                if isinstance(item, collections_abc.MutableMapping):
+                    cd = gather_configtree(item, parse_lists=parse_lists)
+                    cd.parent = out
+                    out.data[k][idx] = cd
+
     return out
 
 
@@ -138,7 +151,7 @@ class ConfigDict(UserDict, object):
         if isinstance(key, str):
             return key.lower().replace('_', '-')
 
-    def accessed_all_keys(self, search='local'):
+    def accessed_all_keys(self, search='local', parse_lists=False):
         """Were all the keys used in the config tree?
 
         Parameters
@@ -156,6 +169,11 @@ class ConfigDict(UserDict, object):
             ``"children"``
                 Recursively check keys in children, moving down the tree, after
                 checking local keys.
+        parse_lists : bool, optional
+            If True when `search` is "children", check if self or its children
+            contain a list and check the list for ConfDicts and whether they
+            used their keys. This is slow. Note this only parses lists,
+            strictly, not all Sequences.
 
         Returns
         -------
@@ -215,17 +233,33 @@ class ConfigDict(UserDict, object):
             # Recursively check parents keys, if any haven't been used,
             # immediately return False.
             if self.parent is not None:
-                parent_used = self.parent.accessed_all_keys(search=search)
+                parent_used = self.parent.accessed_all_keys(
+                    search=search,
+                    parse_lists=parse_lists,
+                )
                 if parent_used is False:
                     return False
+
         elif search == 'children':
             # Recursively check children keys, if any haven't been used,
             # immediately return False.
             for k, v in self.data.items():
-                # Assuming its faster to ask for forgiveness than to check
-                # with `isinstance()` or `hasattr()q...
+                if parse_lists and isinstance(v, list):
+                    for item in v:
+                        try:
+                            child_used = item.accessed_all_keys(
+                                search=search,
+                                parse_lists=parse_lists,
+                            )
+                            if child_used is False:
+                                return False
+                        except AttributeError:
+                            continue
+                    continue
+
                 try:
-                    child_used = v.accessed_all_keys(search=search)
+                    child_used = v.accessed_all_keys(search=search,
+                                                     parse_lists=parse_lists)
                     if child_used is False:
                         return False
                 except AttributeError:


Test patch:
diff --git a/tests/utils/test_configdict.py b/tests/utils/test_configdict.py
index 1d299f0..c26edb8 100644
--- a/tests/utils/test_configdict.py
+++ b/tests/utils/test_configdict.py
@@ -8,6 +8,18 @@ def simple_nested_tree():
     return {'a': 1, 'b': {'a': 2}, 'c': 3, 'd-4': 4, 'e_5': 5, 'F': 6}
 
 
+def test_gather_configtree_nested_lists():
+    """Test gather_configtree() "parse_lists" option"""
+    d = {'a': 1, 'c': [2, {'x': 'foo', 'y': {'z': 'bar'}}]}
+    conf = gather_configtree(d, parse_lists=True)
+    assert conf['c'][1]['y']['a'] == conf['a']
+
+    assert conf.accessed_all_keys(search='children', parse_lists=True) is False
+    conf['c'][1]['x']
+    conf['c'][1]['y']['z']
+    assert conf.accessed_all_keys(search='children', parse_lists=True) is True
+
+
 def test_configdict_climbs_tree(simple_nested_tree):
     conf = gather_configtree(simple_nested_tree)
     assert conf['b']['c'] == 3
@@ -105,24 +117,6 @@ def test_configdict_key_access_stack_nested(simple_nested_tree):
     assert top_keys == ['b', 'f']
 
 
-def test_configdict_accessed_all_keys_local(simple_nested_tree):
-    root_conf = gather_configtree(simple_nested_tree)
-    child_conf = root_conf['b']
-
-    root_conf['a']
-    root_conf['c']
-    root_conf['d-4']
-    root_conf['e-5']
-
-    assert root_conf.accessed_all_keys(search='local') is False
-
-    root_conf['f']
-
-    assert root_conf.accessed_all_keys(search='local') is True
-
-    assert child_conf.accessed_all_keys(search='local') is False
-
-
 def test_configdict_accessed_all_keys_local(simple_nested_tree):
     kwargs = {'search': 'local'}
     root_conf = gather_configtree(simple_nested_tree)