storage test coverage; rework DictStore

Author: alimanfoo

zarr/storage.py

@@ -331,67 +331,64 @@ def __init__(self, cls=dict):
         self.root = cls()
         self.cls = cls
 
-    def __getitem__(self, key):
-        c = self.root
-        for k in key.split('/'):
-            if isinstance(c, self.cls):
-                c = c[k]
-            else:
-                raise KeyError(key)
-        if isinstance(c, self.cls):
-            raise KeyError(key)
-        return c
-
-    def __setitem__(self, key, value):
-        c = self.root
-        keys = key.split('/')
-
-        # ensure intermediate containers
-        for k in keys[:-1]:
-            if isinstance(c, self.cls):
-                try:
-                    c = c[k]
-                except KeyError:
-                    c[k] = self.cls()
-                    c = c[k]
+    def _get_parent(self, item):
+        parent = self.root
+        # split the item
+        segments = item.split('/')
+        # find the parent container
+        for k in segments[:-1]:
+            parent = parent[k]
+            if not isinstance(parent, self.cls):
+                raise KeyError(item)
+        return parent, segments[-1]
+
+    def _require_parent(self, item):
+        parent = self.root
+        # split the item
+        segments = item.split('/')
+        # require the parent container
+        for k in segments[:-1]:
+            try:
+                parent = parent[k]
+            except KeyError:
+                parent[k] = self.cls()
+                parent = parent[k]
             else:
-                raise KeyError(key)
-
-        if isinstance(c, self.cls):
-            # set final value
-            c[keys[-1]] = value
+                if not isinstance(parent, self.cls):
+                    raise KeyError(item)
+        return parent, segments[-1]
+
+    def __getitem__(self, item):
+        parent, key = self._get_parent(item)
+        try:
+            value = parent[key]
+        except KeyError:
+            raise KeyError(item)
         else:
-            raise KeyError(key)
-
-    def __delitem__(self, key):
-        c = self.root
-        keys = key.split('/')
-
-        # obtain final container
-        for k in keys[:-1]:
-            if isinstance(c, self.cls):
-                c = c[k]
+            if isinstance(value, self.cls):
+                raise KeyError(item)
             else:
-                raise KeyError(key)
-
-        if isinstance(c, self.cls):
-            # delete item
-            del c[keys[-1]]
+                return value
+
+    def __setitem__(self, item, value):
+        parent, key = self._require_parent(item)
+        parent[key] = value
+
+    def __delitem__(self, item):
+        parent, key = self._get_parent(item)
+        try:
+            del parent[key]
+        except KeyError:
+            raise KeyError(item)
+
+    def __contains__(self, item):
+        try:
+            parent, key = self._get_parent(item)
+            value = parent[key]
+        except KeyError:
+            return False
         else:
-            raise KeyError(key)
-
-    def __contains__(self, key):
-        keys = key.split('/')
-        c = self.root
-        for k in keys:
-            if isinstance(c, self.cls):
-                try:
-                    c = c[k]
-                except KeyError:
-                    return False
-            else:
-                return False
-        return not isinstance(c, self.cls)
+            return not isinstance(value, self.cls)
 
     def __eq__(self, other):
         return (
@@ -412,46 +409,52 @@ def __len__(self):
 
     def listdir(self, path=None):
         path = normalize_storage_path(path)
-        c = self.root
         if path:
-            # split path and find container
-            for k in path.split('/'):
-                c = c[k]
-        return sorted(c.keys())
+            try:
+                parent, key = self._get_parent(path)
+                value = parent[key]
+            except KeyError:
+                return []
+        else:
+            value = self.root
+        if isinstance(value, self.cls):
+            return sorted(value.keys())
+        else:
+            return []
 
     def rmdir(self, path=None):
         path = normalize_storage_path(path)
-        c = self.root
         if path:
-            # split path and find container
-            segments = path.split('/')
-            for k in segments[:-1]:
-                c = c[k]
-            # remove final key
             try:
-                del c[segments[-1]]
+                parent, key = self._get_parent(path)
+                value = parent[key]
             except KeyError:
-                # does not exist
-                pass
+                return
+            else:
+                if isinstance(value, self.cls):
+                    del parent[key]
         else:
             # clear out root
             self.root = self.cls()
 
     def getsize(self, path=None):
         path = normalize_storage_path(path)
-        c = self.root
+
+        # obtain value to return size of
         if path:
-            # split path and find value
-            segments = path.split('/')
             try:
-                for k in segments:
-                    c = c[k]
+                parent, key = self._get_parent(path)
+                value = parent[key]
             except KeyError:
                 raise ValueError('path not found: %r' % path)
-        if isinstance(c, self.cls):
+        else:
+            value = self.root
+
+        # obtain size of value
+        if isinstance(value, self.cls):
             # total size for directory
             size = 0
-            for v in c.values():
+            for v in value.values():
                 if not isinstance(v, self.cls):
                     try:
                         size += buffersize(v)
@@ -460,7 +463,7 @@ def getsize(self, path=None):
             return size
         else:
             try:
-                return buffersize(c)
+                return buffersize(value)
             except TypeError:
                 return -1
 

zarr/tests/test_storage.py

@@ -156,13 +156,6 @@ def test_hierarchy(self):
         with assert_raises(KeyError):
             store['c/d/x']
 
-        # test listdir (optional)
-        if hasattr(store, 'listdir'):
-            eq({'a', 'b', 'c'}, set(store.listdir()))
-            eq({'d', 'e'}, set(store.listdir('c')))
-            eq({'f', 'g'}, set(store.listdir('c/e')))
-        # TODO further listdir tests
-
         # test getsize (optional)
         if hasattr(store, 'getsize'):
             eq(6, store.getsize())
@@ -173,7 +166,32 @@ def test_hierarchy(self):
             eq(6, store.getsize('c/e'))
             eq(3, store.getsize('c/e/f'))
             eq(3, store.getsize('c/e/g'))
-        # TODO further getsize tests
+            with assert_raises(ValueError):
+                store.getsize('x')
+            with assert_raises(ValueError):
+                store.getsize('a/x')
+            with assert_raises(ValueError):
+                store.getsize('c/x')
+            with assert_raises(ValueError):
+                store.getsize('c/x/y')
+            with assert_raises(ValueError):
+                store.getsize('c/d/y')
+            with assert_raises(ValueError):
+                store.getsize('c/d/y/z')
+
+        # test listdir (optional)
+        if hasattr(store, 'listdir'):
+            eq({'a', 'b', 'c'}, set(store.listdir()))
+            eq({'d', 'e'}, set(store.listdir('c')))
+            eq({'f', 'g'}, set(store.listdir('c/e')))
+            # no exception raised if path does not exist or is leaf
+            eq([], store.listdir('x'))
+            eq([], store.listdir('a/x'))
+            eq([], store.listdir('c/x'))
+            eq([], store.listdir('c/x/y'))
+            eq([], store.listdir('c/d/y'))
+            eq([], store.listdir('c/d/y/z'))
+            eq([], store.listdir('c/e/f'))
 
         # test rmdir (optional)
         if hasattr(store, 'rmdir'):
@@ -186,6 +204,20 @@ def test_hierarchy(self):
             store.rmdir()
             assert 'a' not in store
             assert 'b' not in store
+            store['a'] = b'aaa'
+            store['c/d'] = b'ddd'
+            store['c/e/f'] = b'fff'
+            # no exceptions raised if path does not exist or is leaf
+            store.rmdir('x')
+            store.rmdir('a/x')
+            store.rmdir('c/x')
+            store.rmdir('c/x/y')
+            store.rmdir('c/d/y')
+            store.rmdir('c/d/y/z')
+            store.rmdir('c/e/f')
+            assert 'a' in store
+            assert 'c/d' in store
+            assert 'c/e/f' in store
 
     def test_init_array(self):
         store = self.create_store()
@@ -459,6 +491,14 @@ def test_setdel(self):
         store = self.create_store()
         setdel_hierarchy_checks(store)
 
+    def test_getsize_ext(self):
+        store = self.create_store()
+        store['a'] = list(range(10))
+        store['b/c'] = list(range(10))
+        eq(-1, store.getsize())
+        eq(-1, store.getsize('a'))
+        eq(-1, store.getsize('b'))
+
 
 def rmtree_if_exists(path, rmtree=shutil.rmtree, isdir=os.path.isdir):
     if isdir(path):