Fix clean method not working on nested dicts. #383

Author: fabiocaccamo

benedict/core/clean.py

@@ -1,19 +1,55 @@
-from benedict.utils import type_util
+def _clean_dict(d, strings, collections):
+    keys = list(d.keys())
+    for key in keys:
+        d[key] = _clean_value(d[key], strings=strings, collections=collections)
+        if d[key] is None:
+            del d[key]
+    return d
+
+
+def _clean_list(ls, strings, collections):
+    for i in range(len(ls) - 1, -1, -1):
+        ls[i] = _clean_value(ls[i], strings=strings, collections=collections)
+        if ls[i] is None:
+            ls.pop(i)
+    return ls
+
+
+def _clean_set(values, strings, collections):
+    return {
+        value
+        for value in values
+        if _clean_value(value, strings=strings, collections=collections) is not None
+    }
 
 
-def _clean_item(d, key, strings, collections):
-    value = d.get(key, None)
-    if not value:
-        del_none = value is None
-        del_string = strings and type_util.is_string(value)
-        del_collection = collections and type_util.is_collection(value)
-        return any([del_none, del_string, del_collection])
+def _clean_str(s, strings, collections):
+    return s if s and s.strip() else None
 
-    return False
+
+def _clean_tuple(values, strings, collections):
+    return tuple(
+        value
+        for value in values
+        if _clean_value(value, strings=strings, collections=collections) is not None
+    )
+
+
+def _clean_value(value, strings, collections):
+    if value is None:
+        return value
+    elif isinstance(value, list) and collections:
+        value = _clean_list(value, strings=strings, collections=collections) or None
+    elif isinstance(value, dict) and collections:
+        value = _clean_dict(value, strings=strings, collections=collections) or None
+    elif isinstance(value, set) and collections:
+        value = _clean_set(value, strings=strings, collections=collections) or None
+    elif isinstance(value, str) and strings:
+        value = _clean_str(value, strings=strings, collections=collections) or None
+    elif isinstance(value, tuple) and collections:
+        value = _clean_tuple(value, strings=strings, collections=collections) or None
+    return value
 
 
 def clean(d, strings=True, collections=True):
-    keys = list(d.keys())
-    for key in keys:
-        if _clean_item(d, key, strings, collections):
-            del d[key]
+    return _clean_dict(d, strings=strings, collections=collections)

tests/core/test_clean.py

@@ -18,6 +18,8 @@ def test_clean(self):
             "f": "",
             "g": None,
             "h": "0",
+            "i": (1, None, 2, 3, " "),
+            "j": {1, None, 2, 3, " "},
         }
 
         o = i.copy()
@@ -27,6 +29,8 @@ def test_clean(self):
             "d": [0, 1],
             "e": 0.0,
             "h": "0",
+            "i": (1, 2, 3),
+            "j": {1, 2, 3},
         }
         self.assertEqual(o, r)
 
@@ -39,6 +43,8 @@ def test_clean(self):
             "d": [0, 1],
             "e": 0.0,
             "h": "0",
+            "i": (1, None, 2, 3, " "),
+            "j": {1, None, 2, 3, " "},
         }
         self.assertEqual(o, r)
 
@@ -50,5 +56,90 @@ def test_clean(self):
             "e": 0.0,
             "f": "",
             "h": "0",
+            "i": (1, 2, 3, " "),
+            "j": {1, 2, 3, " "},
         }
         self.assertEqual(o, r)
+
+    def test_clean_nested_dicts(self):
+        # https://github.com/fabiocaccamo/python-benedict/issues/383
+        d = {
+            "a": {
+                "b": {
+                    "c": {},
+                },
+            },
+        }
+        _clean(d, collections=True)
+        r = {}
+        self.assertEqual(d, r)
+
+        d = {
+            "a": {
+                "b": {
+                    "c": {},
+                },
+                "d": 1,
+            },
+        }
+        _clean(d, collections=True)
+        r = {
+            "a": {
+                "d": 1,
+            },
+        }
+        self.assertEqual(d, r)
+
+        d = {
+            "a": {
+                "b": [
+                    0,
+                    1,
+                    2,
+                    3,
+                    {},
+                    {
+                        "c": [None, 4, None, 5],
+                    },
+                ],
+            },
+        }
+        _clean(d, collections=True)
+        r = {
+            "a": {
+                "b": [
+                    0,
+                    1,
+                    2,
+                    3,
+                    {
+                        "c": [4, 5],
+                    },
+                ],
+            },
+        }
+        self.assertEqual(d, r)
+
+        d = {
+            "a": {
+                "b": [
+                    (None, None, None),
+                    (None, 1, 2),
+                    {3, None, 4},
+                    {5, 6, None},
+                ],
+                "c": (None, None),
+                "d": {None},
+            },
+        }
+        _clean(d, collections=True)
+        r = {
+            "a": {
+                "b": [
+                    (1, 2),
+                    {3, 4},
+                    {5, 6},
+                ],
+            },
+        }
+        self.assertEqual(d, r)