Ensure DictStore contains only bytes

docs/release.rst

@@ -35,6 +35,9 @@ Bug fixes
 * Failing tests related to pickling/unpickling have been fixed. By :user:`Ryan Williams <ryan-williams>`,
   :issue:`273`, :issue:`308`.
 
+* Ensure ``DictStore`` contains only ``bytes`` to facilitate comparisons and protect against writes.
+  By :user:`John Kirkham <jakirkham>`, :issue:`350`
+
 Maintenance
 ~~~~~~~~~~~
 

zarr/storage.py

@@ -544,6 +544,7 @@ def __getitem__(self, item):
     def __setitem__(self, item, value):
         with self.write_mutex:
             parent, key = self._require_parent(item)
+            value = ensure_bytes(value)
             parent[key] = value
 
     def __delitem__(self, item):
@@ -642,17 +643,11 @@ def getsize(self, path=None):
             size = 0
             for v in value.values():
                 if not isinstance(v, self.cls):
-                    try:
-                        size += buffer_size(v)
-                    except TypeError:
-                        return -1
+                    size += buffer_size(v)
             return size
 
         else:
-            try:
-                return buffer_size(value)
-            except TypeError:
-                return -1
+            return buffer_size(value)
 
     def clear(self):
         with self.write_mutex:

zarr/tests/test_storage.py

@@ -63,6 +63,12 @@ def test_get_set_del_contains(self):
                 # noinspection PyStatementEffect
                 del store['foo']
 
+    def test_set_invalid_content(self):
+        store = self.create_store()
+
+        with pytest.raises(TypeError):
+            store['baz'] = list(range(5))
+
     def test_clear(self):
         store = self.create_store()
         store['foo'] = b'bar'
@@ -586,6 +592,10 @@ class TestMappingStore(StoreTests, unittest.TestCase):
     def create_store(self):
         return dict()
 
+    def test_set_invalid_content(self):
+        # Generic mappings support non-buffer types
+        pass
+
 
 def setdel_hierarchy_checks(store):
     # these tests are for stores that are aware of hierarchy levels; this
@@ -629,17 +639,14 @@ class TestDictStore(StoreTests, unittest.TestCase):
     def create_store(self):
         return DictStore()
 
-    def test_setdel(self):
+    def test_store_contains_bytes(self):
         store = self.create_store()
-        setdel_hierarchy_checks(store)
+        store['foo'] = np.array([97, 98, 99, 100, 101], dtype=np.uint8)
+        assert store['foo'] == b'abcde'
 
-    def test_getsize_ext(self):
+    def test_setdel(self):
         store = self.create_store()
-        store['a'] = list(range(10))
-        store['b/c'] = list(range(10))
-        assert -1 == store.getsize()
-        assert -1 == store.getsize('a')
-        assert -1 == store.getsize('b')
+        setdel_hierarchy_checks(store)
 
 
 class TestDirectoryStore(StoreTests, unittest.TestCase):
@@ -1096,6 +1103,10 @@ def test_getsize():
     assert 7 == getsize(store)
     assert 5 == getsize(store, 'baz')
 
+    store = dict()
+    store['boo'] = None
+    assert -1 == getsize(store)
+
 
 def test_migrate_1to2():
     from zarr import meta_v1