From 3fa6c3a521d91c52ced2c98b702b6003d08f2281 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Sun, 24 Dec 2017 00:14:41 +0000 Subject: [PATCH 01/13] cache attributes --- zarr/attrs.py | 41 ++++++++++------ zarr/core.py | 8 ++- zarr/tests/test_attrs.py | 102 ++++++++++++++++++++++++++++++++++++++- zarr/tests/test_core.py | 22 ++++++++- zarr/tests/test_sync.py | 10 ++-- 5 files changed, 157 insertions(+), 26 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index f8756debac..2445e63986 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -10,23 +10,42 @@ class Attributes(MutableMapping): - def __init__(self, store, key='.zattrs', read_only=False, + def __init__(self, store, key='.zattrs', read_only=False, cache=True, synchronizer=None): self.store = store self.key = key self.read_only = read_only + self.cache = cache + self._cached_asdict = None self.synchronizer = synchronizer + def _get(self): + if self.key in self.store: + d = json.loads(text_type(self.store[self.key], 'ascii')) + else: + d = dict() + return d + + def _put(self, d): + s = json.dumps(d, indent=4, sort_keys=True, ensure_ascii=True, separators=(',', ': ')) + self.store[self.key] = s.encode('ascii') + if self.cache: + self._cached_asdict = d + + def asdict(self): + if self.cache and self._cached_asdict is not None: + return self._cached_asdict + d = self._get() + if self.cache: + self._cached_asdict = d + return d + def __contains__(self, x): return x in self.asdict() def __getitem__(self, item): return self.asdict()[item] - def _put(self, d): - s = json.dumps(d, indent=4, sort_keys=True, ensure_ascii=True, separators=(',', ': ')) - self.store[self.key] = s.encode('ascii') - def _write_op(self, f, *args, **kwargs): # guard condition @@ -46,7 +65,7 @@ def __setitem__(self, item, value): def _setitem_nosync(self, item, value): # load existing data - d = self.asdict() + d = self._get() # set key value d[item] = value @@ -60,7 +79,7 @@ def __delitem__(self, item): def _delitem_nosync(self, key): # load existing data - d = self.asdict() + d = self._get() # delete key value del d[key] @@ -68,12 +87,6 @@ def _delitem_nosync(self, key): # _put modified data self._put(d) - def asdict(self): - if self.key in self.store: - return json.loads(text_type(self.store[self.key], 'ascii')) - else: - return dict() - def update(self, *args, **kwargs): # override to provide update in a single write self._write_op(self._update_nosync, *args, **kwargs) @@ -81,7 +94,7 @@ def update(self, *args, **kwargs): def _update_nosync(self, *args, **kwargs): # load existing data - d = self.asdict() + d = self._get() # update d.update(*args, **kwargs) diff --git a/zarr/core.py b/zarr/core.py index fa99885e35..ee4f6bf9c9 100644 --- a/zarr/core.py +++ b/zarr/core.py @@ -47,6 +47,10 @@ class Array(object): lifetime of the object. If False, array metadata will be reloaded prior to all data access and modification operations (may incur overhead depending on storage and data access pattern). + cache_attrs : bool, optional + If True (default), user attributes will be cached for attribute read + operations. If False, user attributes are reloaded from the store prior + to all attribute read operations. Attributes ---------- @@ -99,7 +103,7 @@ class Array(object): """ def __init__(self, store, path=None, read_only=False, chunk_store=None, - synchronizer=None, cache_metadata=True): + synchronizer=None, cache_metadata=True, cache_attrs=True): # N.B., expect at this point store is fully initialized with all # configuration metadata fully specified and normalized @@ -121,7 +125,7 @@ def __init__(self, store, path=None, read_only=False, chunk_store=None, # initialize attributes akey = self._key_prefix + attrs_key self._attrs = Attributes(store, key=akey, read_only=read_only, - synchronizer=synchronizer) + synchronizer=synchronizer, cache=cache_attrs) # initialize info reporter self._info_reporter = InfoReporter(self) diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index 33f9a9e236..dac62bec85 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -2,6 +2,7 @@ from __future__ import absolute_import, print_function, division import json import unittest +import collections from nose.tools import eq_ as eq, assert_raises @@ -12,10 +13,38 @@ from zarr.errors import PermissionError +class CountingDict(collections.MutableMapping): + + def __init__(self): + self.wrapped = dict() + self.counter = collections.Counter() + + def __len__(self): + return len(self.wrapped) + + def __iter__(self): + return iter(self.wrapped) + + def __contains__(self, item): + return item in self.wrapped + + def __getitem__(self, item): + self.counter['__getitem__', item] += 1 + return self.wrapped[item] + + def __setitem__(self, key, value): + self.counter['__setitem__', key] += 1 + self.wrapped[key] = value + + def __delitem__(self, key): + self.counter['__delitem__', key] += 1 + del self.wrapped[key] + + class TestAttributes(unittest.TestCase): - def init_attributes(self, store, read_only=False): - return Attributes(store, key='attrs', read_only=read_only) + def init_attributes(self, store, read_only=False, cache=True): + return Attributes(store, key='attrs', read_only=read_only, cache=cache) def test_storage(self): @@ -102,3 +131,72 @@ def test_key_completions(self): assert '123' in d assert 'asdf;' in d assert 'baz' not in d + + def test_caching_on(self): + # caching is turned on by default + store = CountingDict() + eq(0, store.counter['__getitem__', 'attrs']) + eq(0, store.counter['__setitem__', 'attrs']) + store['attrs'] = json.dumps(dict(foo='xxx', bar=42)).encode('ascii') + eq(0, store.counter['__getitem__', 'attrs']) + eq(1, store.counter['__setitem__', 'attrs']) + a = self.init_attributes(store) + eq(a['foo'], 'xxx') + eq(1, store.counter['__getitem__', 'attrs']) + eq(a['bar'], 42) + eq(1, store.counter['__getitem__', 'attrs']) + eq(a['foo'], 'xxx') + eq(1, store.counter['__getitem__', 'attrs']) + a['foo'] = 'yyy' + eq(2, store.counter['__getitem__', 'attrs']) + eq(2, store.counter['__setitem__', 'attrs']) + eq(a['foo'], 'yyy') + eq(2, store.counter['__getitem__', 'attrs']) + eq(2, store.counter['__setitem__', 'attrs']) + a.update(foo='zzz', bar=84) + eq(3, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + eq(a['foo'], 'zzz') + eq(a['bar'], 84) + eq(3, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + assert 'foo' in a + eq(3, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + assert 'spam' not in a + eq(3, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + + def test_caching_off(self): + store = CountingDict() + eq(0, store.counter['__getitem__', 'attrs']) + eq(0, store.counter['__setitem__', 'attrs']) + store['attrs'] = json.dumps(dict(foo='xxx', bar=42)).encode('ascii') + eq(0, store.counter['__getitem__', 'attrs']) + eq(1, store.counter['__setitem__', 'attrs']) + a = self.init_attributes(store, cache=False) + eq(a['foo'], 'xxx') + eq(1, store.counter['__getitem__', 'attrs']) + eq(a['bar'], 42) + eq(2, store.counter['__getitem__', 'attrs']) + eq(a['foo'], 'xxx') + eq(3, store.counter['__getitem__', 'attrs']) + a['foo'] = 'yyy' + eq(4, store.counter['__getitem__', 'attrs']) + eq(2, store.counter['__setitem__', 'attrs']) + eq(a['foo'], 'yyy') + eq(5, store.counter['__getitem__', 'attrs']) + eq(2, store.counter['__setitem__', 'attrs']) + a.update(foo='zzz', bar=84) + eq(6, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + eq(a['foo'], 'zzz') + eq(a['bar'], 84) + eq(8, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + assert 'foo' in a + eq(9, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) + assert 'spam' not in a + eq(10, store.counter['__getitem__', 'attrs']) + eq(3, store.counter['__setitem__', 'attrs']) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index c50328422e..e3cd1a31ce 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -1565,14 +1565,15 @@ def test_nbytes_stored(self): eq(-1, z.nbytes_stored) -class TestArrayNoCacheMetadata(TestArray): +class TestArrayNoCache(TestArray): @staticmethod def create_array(read_only=False, **kwargs): store = dict() kwargs.setdefault('compressor', Zlib(level=1)) init_array(store, **kwargs) - return Array(store, read_only=read_only, cache_metadata=False) + return Array(store, read_only=read_only, cache_metadata=False, + cache_attrs=False) def test_cache_metadata(self): a1 = self.create_array(shape=100, chunks=10, dtype='i1') @@ -1582,6 +1583,7 @@ def test_cache_metadata(self): eq(a1.nbytes, a2.nbytes) eq(a1.nchunks, a2.nchunks) + # a1 is not caching so *will* see updates made via other objects a2.resize(200) eq((200,), a2.shape) eq(200, a2.size) @@ -1602,6 +1604,7 @@ def test_cache_metadata(self): eq(a1.nbytes, a2.nbytes) eq(a1.nchunks, a2.nchunks) + # a2 is caching so *will not* see updates made via other objects a1.resize(400) eq((400,), a1.shape) eq(400, a1.size) @@ -1612,6 +1615,21 @@ def test_cache_metadata(self): eq(300, a2.nbytes) eq(30, a2.nchunks) + def test_cache_attrs(self): + a1 = self.create_array(shape=100, chunks=10, dtype='i1') + a2 = Array(a1.store, cache_attrs=True) + eq(a1.attrs.asdict(), a2.attrs.asdict()) + + # a1 is not caching so *will* see updates made via other objects + a2.attrs['foo'] = 'xxx' + a2.attrs['bar'] = 42 + eq(a1.attrs.asdict(), a2.attrs.asdict()) + + # a2 is caching so *will not* see updates made via other objects + a1.attrs['foo'] = 'yyy' + assert 'yyy' == a1.attrs['foo'] + assert 'xxx' == a2.attrs['foo'] + def test_object_arrays_danger(self): # skip this one as it only works if metadata are cached pass diff --git a/zarr/tests/test_sync.py b/zarr/tests/test_sync.py index d9a664c72c..b23cbd5f2a 100644 --- a/zarr/tests/test_sync.py +++ b/zarr/tests/test_sync.py @@ -27,24 +27,22 @@ class TestAttributesWithThreadSynchronizer(TestAttributes): - def init_attributes(self, store, read_only=False): + def init_attributes(self, store, read_only=False, cache=True): key = 'attrs' - store[key] = json.dumps(dict()).encode('ascii') synchronizer = ThreadSynchronizer() return Attributes(store, synchronizer=synchronizer, key=key, - read_only=read_only) + read_only=read_only, cache=cache) class TestAttributesProcessSynchronizer(TestAttributes): - def init_attributes(self, store, read_only=False): + def init_attributes(self, store, read_only=False, cache=True): key = 'attrs' - store[key] = json.dumps(dict()).encode('ascii') sync_path = mkdtemp() atexit.register(shutil.rmtree, sync_path) synchronizer = ProcessSynchronizer(sync_path) return Attributes(store, synchronizer=synchronizer, key=key, - read_only=read_only) + read_only=read_only, cache=cache) def _append(arg): From 89c1ea33779a1b5ecf8636fbf3c452f4209e328a Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Sun, 24 Dec 2017 00:28:31 +0000 Subject: [PATCH 02/13] lint --- zarr/tests/test_sync.py | 1 - 1 file changed, 1 deletion(-) diff --git a/zarr/tests/test_sync.py b/zarr/tests/test_sync.py index b23cbd5f2a..0dee2ba397 100644 --- a/zarr/tests/test_sync.py +++ b/zarr/tests/test_sync.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, print_function, division from tempfile import mkdtemp import atexit -import json import shutil from multiprocessing.pool import ThreadPool from multiprocessing import Pool as ProcessPool From 1d502c043d0358a8139826b439462291b35ea907 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Sat, 30 Dec 2017 22:48:16 +0000 Subject: [PATCH 03/13] factor out counting dict --- zarr/tests/test_attrs.py | 30 +----------------------------- zarr/tests/util.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 29 deletions(-) create mode 100644 zarr/tests/util.py diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index dac62bec85..cbfac2ebb3 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -2,7 +2,6 @@ from __future__ import absolute_import, print_function, division import json import unittest -import collections from nose.tools import eq_ as eq, assert_raises @@ -11,34 +10,7 @@ from zarr.attrs import Attributes from zarr.compat import binary_type, text_type from zarr.errors import PermissionError - - -class CountingDict(collections.MutableMapping): - - def __init__(self): - self.wrapped = dict() - self.counter = collections.Counter() - - def __len__(self): - return len(self.wrapped) - - def __iter__(self): - return iter(self.wrapped) - - def __contains__(self, item): - return item in self.wrapped - - def __getitem__(self, item): - self.counter['__getitem__', item] += 1 - return self.wrapped[item] - - def __setitem__(self, key, value): - self.counter['__setitem__', key] += 1 - self.wrapped[key] = value - - def __delitem__(self, key): - self.counter['__delitem__', key] += 1 - del self.wrapped[key] +from zarr.tests.util import CountingDict class TestAttributes(unittest.TestCase): diff --git a/zarr/tests/util.py b/zarr/tests/util.py new file mode 100644 index 0000000000..5fcee7ea64 --- /dev/null +++ b/zarr/tests/util.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import, print_function, division +import collections + + +class CountingDict(collections.MutableMapping): + + def __init__(self): + self.wrapped = dict() + self.counter = collections.Counter() + + def __len__(self): + return len(self.wrapped) + + def __iter__(self): + return iter(self.wrapped) + + def __contains__(self, item): + return item in self.wrapped + + def __getitem__(self, item): + self.counter['__getitem__', item] += 1 + return self.wrapped[item] + + def __setitem__(self, key, value): + self.counter['__setitem__', key] += 1 + self.wrapped[key] = value + + def __delitem__(self, key): + self.counter['__delitem__', key] += 1 + del self.wrapped[key] From 1f66707a69138b024eecfb248d84715612ebf52b Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 12:32:35 +0000 Subject: [PATCH 04/13] ask for forgiveness --- zarr/attrs.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index 2445e63986..1584bc5b2d 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -20,10 +20,12 @@ def __init__(self, store, key='.zattrs', read_only=False, cache=True, self.synchronizer = synchronizer def _get(self): - if self.key in self.store: - d = json.loads(text_type(self.store[self.key], 'ascii')) - else: + try: + data = self.store[self.key] + except KeyError: d = dict() + else: + d = json.loads(text_type(data, 'ascii')) return d def _put(self, d): From 14f218976a886b037a98c796377e81144e145c8b Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 12:42:42 +0000 Subject: [PATCH 05/13] attributes method naming, expose put() --- zarr/attrs.py | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index 1584bc5b2d..29e46dcb11 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -9,6 +9,9 @@ class Attributes(MutableMapping): + """Class providing access to user attributes on an array or group. Should not be + instantiated directly, will be available via the `.attrs` property of an array or + group.""" def __init__(self, store, key='.zattrs', read_only=False, cache=True, synchronizer=None): @@ -19,7 +22,7 @@ def __init__(self, store, key='.zattrs', read_only=False, cache=True, self._cached_asdict = None self.synchronizer = synchronizer - def _get(self): + def _get_nosync(self): try: data = self.store[self.key] except KeyError: @@ -28,16 +31,11 @@ def _get(self): d = json.loads(text_type(data, 'ascii')) return d - def _put(self, d): - s = json.dumps(d, indent=4, sort_keys=True, ensure_ascii=True, separators=(',', ': ')) - self.store[self.key] = s.encode('ascii') - if self.cache: - self._cached_asdict = d - def asdict(self): + """Retrieve all attributes as a dictionary.""" if self.cache and self._cached_asdict is not None: return self._cached_asdict - d = self._get() + d = self._get_nosync() if self.cache: self._cached_asdict = d return d @@ -67,13 +65,13 @@ def __setitem__(self, item, value): def _setitem_nosync(self, item, value): # load existing data - d = self._get() + d = self._get_nosync() # set key value d[item] = value # _put modified data - self._put(d) + self._put_nosync(d) def __delitem__(self, item): self._write_op(self._delitem_nosync, item) @@ -81,29 +79,43 @@ def __delitem__(self, item): def _delitem_nosync(self, key): # load existing data - d = self._get() + d = self._get_nosync() # delete key value del d[key] # _put modified data - self._put(d) + self._put_nosync(d) + + def put(self, d): + """Overwrite all attributes with the key/value pairs in the provided dictionary + `d` in a single operation.""" + self._write_op(self._put_nosync, d) + + def _put_nosync(self, d): + s = json.dumps(d, indent=4, sort_keys=True, ensure_ascii=True, separators=(',', ': ')) + self.store[self.key] = s.encode('ascii') + if self.cache: + self._cached_asdict = d def update(self, *args, **kwargs): - # override to provide update in a single write + """Update the values of several attributes in a single operation.""" self._write_op(self._update_nosync, *args, **kwargs) def _update_nosync(self, *args, **kwargs): # load existing data - d = self._get() + d = self._get_nosync() # update d.update(*args, **kwargs) # _put modified data - self._put(d) + self._put_nosync(d) + def keys(self): + return self.asdict().keys() + def __iter__(self): return iter(self.asdict()) From bbc6da6398142b29514eb8758103b607604e3506 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 12:46:52 +0000 Subject: [PATCH 06/13] test put attributes --- zarr/attrs.py | 2 +- zarr/tests/test_attrs.py | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index 29e46dcb11..18ef3e8691 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -115,7 +115,7 @@ def _update_nosync(self, *args, **kwargs): def keys(self): return self.asdict().keys() - + def __iter__(self): return iter(self.asdict()) diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index cbfac2ebb3..4c55875c0a 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -46,16 +46,25 @@ def test_get_set_del_contains(self): del a['foo'] assert 'foo' not in a with assert_raises(KeyError): + # noinspection PyStatementEffect a['foo'] - def test_update(self): + def test_update_put(self): a = self.init_attributes(dict()) assert 'foo' not in a + assert 'bar' not in a + assert 'baz' not in a + + a.update(foo='spam', bar=42, baz=4.2) + eq(a['foo'], 'spam') + eq(a['bar'], 42) + eq(a['baz'], 4.2) + + a.put(dict(foo='eggs', bar=84)) + eq(a['foo'], 'eggs') + eq(a['bar'], 84) assert 'baz' not in a - a.update(foo='bar', baz=42) - eq(a['foo'], 'bar') - eq(a['baz'], 42) def test_iterators(self): From 18f0dc1227f3ac1daa71d82f1d4097bc941f3d08 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 16:29:13 +0000 Subject: [PATCH 07/13] document attributes --- docs/api.rst | 3 ++- docs/api/attrs.rst | 15 +++++++++++++++ zarr/attrs.py | 17 ++++++++++++++++- zarr/tests/test_attrs.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 docs/api/attrs.rst diff --git a/docs/api.rst b/docs/api.rst index f8d5a41897..1e4015460b 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -8,6 +8,7 @@ API reference api/core api/hierarchy api/storage + api/convenience api/codecs + api/attrs api/sync - api/convenience diff --git a/docs/api/attrs.rst b/docs/api/attrs.rst new file mode 100644 index 0000000000..a22914b7d6 --- /dev/null +++ b/docs/api/attrs.rst @@ -0,0 +1,15 @@ +The Attributes class (``zarr.attrs``) +===================================== +.. module:: zarr.attrs + +.. autoclass:: Attributes + + .. automethod:: __getitem__ + .. automethod:: __setitem__ + .. automethod:: __delitem__ + .. automethod:: __iter__ + .. automethod:: __len__ + .. automethod:: keys + .. automethod:: asdict + .. automethod:: put + .. automethod:: update diff --git a/zarr/attrs.py b/zarr/attrs.py index 18ef3e8691..c41c6e970b 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -11,7 +11,22 @@ class Attributes(MutableMapping): """Class providing access to user attributes on an array or group. Should not be instantiated directly, will be available via the `.attrs` property of an array or - group.""" + group. + + Parameters + ---------- + store : MutableMapping + The store in which to store the attributes. + key : str, optional + The key under which the attributes will be stored. + read_only : bool, optional + If True, attributes cannot be modified. + cache : bool, optional + If True (default), attributes will be cached locally. + synchronizer : Synchronizer + Only necessary if attributes may be modified from multiple threads or processes. + + """ def __init__(self, store, key='.zattrs', read_only=False, cache=True, synchronizer=None): diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index 4c55875c0a..e869927b47 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -115,25 +115,35 @@ def test_key_completions(self): def test_caching_on(self): # caching is turned on by default + + # setup store store = CountingDict() eq(0, store.counter['__getitem__', 'attrs']) eq(0, store.counter['__setitem__', 'attrs']) store['attrs'] = json.dumps(dict(foo='xxx', bar=42)).encode('ascii') eq(0, store.counter['__getitem__', 'attrs']) eq(1, store.counter['__setitem__', 'attrs']) + + # setup attributes a = self.init_attributes(store) + + # test __getitem__ causes all attributes to be cached eq(a['foo'], 'xxx') eq(1, store.counter['__getitem__', 'attrs']) eq(a['bar'], 42) eq(1, store.counter['__getitem__', 'attrs']) eq(a['foo'], 'xxx') eq(1, store.counter['__getitem__', 'attrs']) + + # test __setitem__ updates the cache a['foo'] = 'yyy' eq(2, store.counter['__getitem__', 'attrs']) eq(2, store.counter['__setitem__', 'attrs']) eq(a['foo'], 'yyy') eq(2, store.counter['__getitem__', 'attrs']) eq(2, store.counter['__setitem__', 'attrs']) + + # test update() updates the cache a.update(foo='zzz', bar=84) eq(3, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) @@ -141,6 +151,8 @@ def test_caching_on(self): eq(a['bar'], 84) eq(3, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) + + # test __contains__ uses the cache assert 'foo' in a eq(3, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) @@ -148,26 +160,44 @@ def test_caching_on(self): eq(3, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) + # test __delitem__ updates the cache + del a['bar'] + eq(4, store.counter['__getitem__', 'attrs']) + eq(4, store.counter['__setitem__', 'attrs']) + assert 'bar' not in a + eq(4, store.counter['__getitem__', 'attrs']) + eq(4, store.counter['__setitem__', 'attrs']) + def test_caching_off(self): + + # setup store store = CountingDict() eq(0, store.counter['__getitem__', 'attrs']) eq(0, store.counter['__setitem__', 'attrs']) store['attrs'] = json.dumps(dict(foo='xxx', bar=42)).encode('ascii') eq(0, store.counter['__getitem__', 'attrs']) eq(1, store.counter['__setitem__', 'attrs']) + + # setup attributes a = self.init_attributes(store, cache=False) + + # test __getitem__ eq(a['foo'], 'xxx') eq(1, store.counter['__getitem__', 'attrs']) eq(a['bar'], 42) eq(2, store.counter['__getitem__', 'attrs']) eq(a['foo'], 'xxx') eq(3, store.counter['__getitem__', 'attrs']) + + # test __setitem__ a['foo'] = 'yyy' eq(4, store.counter['__getitem__', 'attrs']) eq(2, store.counter['__setitem__', 'attrs']) eq(a['foo'], 'yyy') eq(5, store.counter['__getitem__', 'attrs']) eq(2, store.counter['__setitem__', 'attrs']) + + # test update() a.update(foo='zzz', bar=84) eq(6, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) @@ -175,6 +205,8 @@ def test_caching_off(self): eq(a['bar'], 84) eq(8, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) + + # test __contains__ assert 'foo' in a eq(9, store.counter['__getitem__', 'attrs']) eq(3, store.counter['__setitem__', 'attrs']) From 5fc83859ab996091e3368233e16c5a3605ee446d Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 16:51:34 +0000 Subject: [PATCH 08/13] add cache_attrs to API --- zarr/core.py | 2 +- zarr/creation.py | 21 ++++++++++++------ zarr/hierarchy.py | 54 +++++++++++++++++++++++++++++++++++++---------- 3 files changed, 59 insertions(+), 18 deletions(-) diff --git a/zarr/core.py b/zarr/core.py index ee4f6bf9c9..db33e7bef1 100644 --- a/zarr/core.py +++ b/zarr/core.py @@ -43,7 +43,7 @@ class Array(object): synchronizer : object, optional Array synchronizer. cache_metadata : bool, optional - If True, array configuration metadata will be cached for the + If True (default), array configuration metadata will be cached for the lifetime of the object. If False, array metadata will be reloaded prior to all data access and modification operations (may incur overhead depending on storage and data access pattern). diff --git a/zarr/creation.py b/zarr/creation.py index d7241ad3c5..b16cae6125 100644 --- a/zarr/creation.py +++ b/zarr/creation.py @@ -16,8 +16,8 @@ def create(shape, chunks=True, dtype=None, compressor='default', fill_value=0, order='C', store=None, synchronizer=None, overwrite=False, path=None, chunk_store=None, filters=None, - cache_metadata=True, read_only=False, object_codec=None, - **kwargs): + cache_metadata=True, cache_attrs=True, read_only=False, + object_codec=None, **kwargs): """Create an array. Parameters @@ -54,6 +54,10 @@ def create(shape, chunks=True, dtype=None, compressor='default', lifetime of the object. If False, array metadata will be reloaded prior to all data access and modification operations (may incur overhead depending on storage and data access pattern). + cache_attrs : bool, optional + If True (default), user attributes will be cached for attribute read + operations. If False, user attributes are reloaded from the store prior + to all attribute read operations. read_only : bool, optional True if array should be protected against modification. object_codec : Codec, optional @@ -115,7 +119,7 @@ def create(shape, chunks=True, dtype=None, compressor='default', # instantiate array z = Array(store, path=path, chunk_store=chunk_store, synchronizer=synchronizer, - cache_metadata=cache_metadata, read_only=read_only) + cache_metadata=cache_metadata, cache_attrs=cache_attrs, read_only=read_only) return z @@ -342,8 +346,9 @@ def array(data, **kwargs): def open_array(store, mode='a', shape=None, chunks=True, dtype=None, compressor='default', - fill_value=0, order='C', synchronizer=None, filters=None, cache_metadata=True, - path=None, object_codec=None, **kwargs): + fill_value=0, order='C', synchronizer=None, filters=None, + cache_metadata=True, cache_attrs=True, path=None, object_codec=None, + **kwargs): """Open an array using file-mode-like semantics. Parameters @@ -377,6 +382,10 @@ def open_array(store, mode='a', shape=None, chunks=True, dtype=None, compressor= lifetime of the object. If False, array metadata will be reloaded prior to all data access and modification operations (may incur overhead depending on storage and data access pattern). + cache_attrs : bool, optional + If True (default), user attributes will be cached for attribute read + operations. If False, user attributes are reloaded from the store prior + to all attribute read operations. path : string, optional Array path within store. object_codec : Codec, optional @@ -465,7 +474,7 @@ def open_array(store, mode='a', shape=None, chunks=True, dtype=None, compressor= # instantiate array z = Array(store, read_only=read_only, synchronizer=synchronizer, - cache_metadata=cache_metadata, path=path) + cache_metadata=cache_metadata, cache_attrs=cache_attrs, path=path) return z diff --git a/zarr/hierarchy.py b/zarr/hierarchy.py index 02756409b8..a1f3f20ea8 100644 --- a/zarr/hierarchy.py +++ b/zarr/hierarchy.py @@ -35,6 +35,10 @@ class Group(MutableMapping): chunk_store : MutableMapping, optional Separate storage for chunks. If not provided, `store` will be used for storage of both chunks and metadata. + cache_attrs : bool, optional + If True (default), user attributes will be cached for attribute read + operations. If False, user attributes are reloaded from the store prior + to all attribute read operations. synchronizer : object, optional Array synchronizer. @@ -86,7 +90,7 @@ class Group(MutableMapping): """ def __init__(self, store, path=None, read_only=False, chunk_store=None, - synchronizer=None): + cache_attrs=True, synchronizer=None): self._store = store self._chunk_store = chunk_store @@ -115,7 +119,7 @@ def __init__(self, store, path=None, read_only=False, chunk_store=None, # setup attributes akey = self._key_prefix + attrs_key self._attrs = Attributes(store, key=akey, read_only=read_only, - synchronizer=synchronizer) + cache=cache_attrs, synchronizer=synchronizer) # setup info self._info = InfoReporter(self) @@ -320,10 +324,12 @@ def __getitem__(self, item): path = self._item_path(item) if contains_array(self._store, path): return Array(self._store, read_only=self._read_only, path=path, - chunk_store=self._chunk_store, synchronizer=self._synchronizer) + chunk_store=self._chunk_store, + synchronizer=self._synchronizer, cache_attrs=self.attrs.cache) elif contains_group(self._store, path): return Group(self._store, read_only=self._read_only, path=path, - chunk_store=self._chunk_store, synchronizer=self._synchronizer) + chunk_store=self._chunk_store, cache_attrs=self.attrs.cache, + synchronizer=self._synchronizer) else: raise KeyError(item) @@ -403,6 +409,7 @@ def groups(self): if contains_group(self._store, path): yield key, Group(self._store, path=path, read_only=self._read_only, chunk_store=self._chunk_store, + cache_attrs=self.attrs.cache, synchronizer=self._synchronizer) def array_keys(self): @@ -447,6 +454,7 @@ def arrays(self): if contains_array(self._store, path): yield key, Array(self._store, path=path, read_only=self._read_only, chunk_store=self._chunk_store, + cache_attrs=self.attrs.cache, synchronizer=self._synchronizer) def visitvalues(self, func): @@ -649,7 +657,8 @@ def _create_group_nosync(self, name, overwrite=False): overwrite=overwrite) return Group(self._store, path=path, read_only=self._read_only, - chunk_store=self._chunk_store, synchronizer=self._synchronizer) + chunk_store=self._chunk_store, cache_attrs=self.attrs.cache, + synchronizer=self._synchronizer) def create_groups(self, *names, **kwargs): """Convenience method to create multiple groups in a single call.""" @@ -692,7 +701,8 @@ def _require_group_nosync(self, name, overwrite=False): overwrite=overwrite) return Group(self._store, path=path, read_only=self._read_only, - chunk_store=self._chunk_store, synchronizer=self._synchronizer) + chunk_store=self._chunk_store, cache_attrs=self.attrs.cache, + synchronizer=self._synchronizer) def require_groups(self, *names): """Convenience method to require multiple groups in a single call.""" @@ -760,6 +770,7 @@ def _create_dataset_nosync(self, name, data=None, **kwargs): # determine synchronizer kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) # create array if data is None: @@ -804,9 +815,10 @@ def _require_dataset_nosync(self, name, shape, dtype=None, exact=False, synchronizer = kwargs.get('synchronizer', self._synchronizer) cache_metadata = kwargs.get('cache_metadata', True) + cache_attrs = kwargs.get('cache_attrs', self.attrs.cache) a = Array(self._store, path=path, read_only=self._read_only, chunk_store=self._chunk_store, synchronizer=synchronizer, - cache_metadata=cache_metadata) + cache_metadata=cache_metadata, cache_attrs=cache_attrs) shape = normalize_shape(shape) if shape != a.shape: raise TypeError('shape do not match existing array; expected {}, got {}' @@ -834,6 +846,7 @@ def create(self, name, **kwargs): def _create_nosync(self, name, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return create(store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -845,6 +858,7 @@ def empty(self, name, **kwargs): def _empty_nosync(self, name, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return empty(store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -856,6 +870,7 @@ def zeros(self, name, **kwargs): def _zeros_nosync(self, name, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return zeros(store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -867,6 +882,7 @@ def ones(self, name, **kwargs): def _ones_nosync(self, name, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return ones(store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) def full(self, name, fill_value, **kwargs): @@ -877,6 +893,7 @@ def full(self, name, fill_value, **kwargs): def _full_nosync(self, name, fill_value, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return full(store=self._store, path=path, chunk_store=self._chunk_store, fill_value=fill_value, **kwargs) @@ -888,6 +905,7 @@ def array(self, name, data, **kwargs): def _array_nosync(self, name, data, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return array(data, store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -899,6 +917,7 @@ def empty_like(self, name, data, **kwargs): def _empty_like_nosync(self, name, data, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return empty_like(data, store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -910,6 +929,7 @@ def zeros_like(self, name, data, **kwargs): def _zeros_like_nosync(self, name, data, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return zeros_like(data, store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -921,6 +941,7 @@ def ones_like(self, name, data, **kwargs): def _ones_like_nosync(self, name, data, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return ones_like(data, store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -932,6 +953,7 @@ def full_like(self, name, data, **kwargs): def _full_like_nosync(self, name, data, **kwargs): path = self._item_path(name) kwargs.setdefault('synchronizer', self._synchronizer) + kwargs.setdefault('cache_attrs', self.attrs.cache) return full_like(data, store=self._store, path=path, chunk_store=self._chunk_store, **kwargs) @@ -971,7 +993,8 @@ def _normalize_store_arg(store, clobber=False): return normalize_store_arg(store, clobber=clobber, default=DictStore) -def group(store=None, overwrite=False, chunk_store=None, synchronizer=None, path=None): +def group(store=None, overwrite=False, chunk_store=None, + cache_attrs=True, synchronizer=None, path=None): """Create a group. Parameters @@ -984,6 +1007,10 @@ def group(store=None, overwrite=False, chunk_store=None, synchronizer=None, path chunk_store : MutableMapping, optional Separate storage for chunks. If not provided, `store` will be used for storage of both chunks and metadata. + cache_attrs : bool, optional + If True (default), user attributes will be cached for attribute read + operations. If False, user attributes are reloaded from the store prior + to all attribute read operations. synchronizer : object, optional Array synchronizer. path : string, optional @@ -1021,10 +1048,10 @@ def group(store=None, overwrite=False, chunk_store=None, synchronizer=None, path path=path) return Group(store, read_only=False, chunk_store=chunk_store, - synchronizer=synchronizer, path=path) + cache_attrs=cache_attrs, synchronizer=synchronizer, path=path) -def open_group(store, mode='a', synchronizer=None, path=None): +def open_group(store, mode='a', cache_attrs=True, synchronizer=None, path=None): """Open a group using file-mode-like semantics. Parameters @@ -1036,6 +1063,10 @@ def open_group(store, mode='a', synchronizer=None, path=None): read/write (must exist); 'a' means read/write (create if doesn't exist); 'w' means create (overwrite if exists); 'w-' means create (fail if exists). + cache_attrs : bool, optional + If True (default), user attributes will be cached for attribute read + operations. If False, user attributes are reloaded from the store prior + to all attribute read operations. synchronizer : object, optional Array synchronizer. path : string, optional @@ -1093,4 +1124,5 @@ def open_group(store, mode='a', synchronizer=None, path=None): # determine read only status read_only = mode == 'r' - return Group(store, read_only=read_only, synchronizer=synchronizer, path=path) + return Group(store, read_only=read_only, cache_attrs=cache_attrs, + synchronizer=synchronizer, path=path) From d4eb2fd4aaf6dac936377da5f4c6ce25c7d7c5f5 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 16:59:49 +0000 Subject: [PATCH 09/13] fix bug in group pickle/unpickle --- zarr/hierarchy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zarr/hierarchy.py b/zarr/hierarchy.py index a1f3f20ea8..2e0633d77d 100644 --- a/zarr/hierarchy.py +++ b/zarr/hierarchy.py @@ -267,7 +267,7 @@ def typestr(o): def __getstate__(self): return (self._store, self._path, self._read_only, self._chunk_store, - self._synchronizer) + self.attrs.cache, self._synchronizer) def __setstate__(self, state): self.__init__(*state) From 3a4dfa4daafc262af02144663265763993af1d40 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 18:13:39 +0000 Subject: [PATCH 10/13] fix bug in Array pickle/unpickle with cache_attrs --- zarr/core.py | 2 +- zarr/tests/test_core.py | 90 ++++++++++++++++++++++++++++++++--------- zarr/tests/test_sync.py | 11 +++-- 3 files changed, 79 insertions(+), 24 deletions(-) diff --git a/zarr/core.py b/zarr/core.py index db33e7bef1..03d9bdc667 100644 --- a/zarr/core.py +++ b/zarr/core.py @@ -1924,7 +1924,7 @@ def hexdigest(self, hashname="sha1"): def __getstate__(self): return (self._store, self._path, self._read_only, self._chunk_store, - self._synchronizer, self._cache_metadata) + self._synchronizer, self._cache_metadata, self._attrs.cache) def __setstate__(self, state): self.__init__(*state) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index e3cd1a31ce..83c7808fc9 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -79,8 +79,11 @@ def test_array_init(self): def create_array(self, read_only=False, **kwargs): store = dict() kwargs.setdefault('compressor', Zlib(level=1)) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_nbytes_stored(self): @@ -655,7 +658,8 @@ def test_read_only(self): def test_pickle(self): - z = self.create_array(shape=1000, chunks=100, dtype=int) + z = self.create_array(shape=1000, chunks=100, dtype=int, cache_metadata=False, + cache_attrs=False) z[:] = np.random.randint(0, 1000, 1000) z2 = pickle.loads(pickle.dumps(z)) eq(z.shape, z2.shape) @@ -664,6 +668,8 @@ def test_pickle(self): if z.compressor: eq(z.compressor.get_config(), z2.compressor.get_config()) eq(z.fill_value, z2.fill_value) + eq(z._cache_metadata, z2._cache_metadata) + eq(z.attrs.cache, z2.attrs.cache) assert_array_equal(z[:], z2[:]) def test_np_ufuncs(self): @@ -1081,8 +1087,11 @@ class TestArrayWithPath(TestArray): @staticmethod def create_array(read_only=False, **kwargs): store = dict() + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, path='foo/bar', **kwargs) - return Array(store, path='foo/bar', read_only=read_only) + return Array(store, path='foo/bar', read_only=read_only, + cache_metadata=cache_metadata, cache_attrs=cache_attrs) def test_hexdigest(self): # Check basic 1-D array @@ -1133,8 +1142,11 @@ def create_array(read_only=False, **kwargs): store = dict() # separate chunk store chunk_store = dict() + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, chunk_store=chunk_store, **kwargs) - return Array(store, read_only=read_only, chunk_store=chunk_store) + return Array(store, read_only=read_only, chunk_store=chunk_store, + cache_metadata=cache_metadata, cache_attrs=cache_attrs) def test_hexdigest(self): # Check basic 1-D array @@ -1184,9 +1196,12 @@ def create_array(read_only=False, **kwargs): path = mkdtemp() atexit.register(shutil.rmtree, path) store = DirectoryStore(path) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) kwargs.setdefault('compressor', Zlib(1)) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_nbytes_stored(self): @@ -1206,9 +1221,12 @@ def create_array(read_only=False, **kwargs): path = mkdtemp() atexit.register(shutil.rmtree, path) store = NestedDirectoryStore(path) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) kwargs.setdefault('compressor', Zlib(1)) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) class TestArrayWithDBMStore(TestArray): @@ -1218,9 +1236,12 @@ def create_array(read_only=False, **kwargs): path = mktemp(suffix='.anydbm') atexit.register(atexit_rmglob, path + '*') store = DBMStore(path, flag='n') + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) kwargs.setdefault('compressor', Zlib(1)) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_attrs=cache_attrs, + cache_metadata=cache_metadata) def test_nbytes_stored(self): pass # not implemented @@ -1236,9 +1257,12 @@ def create_array(read_only=False, **kwargs): path = mktemp(suffix='.dbm') atexit.register(os.remove, path) store = DBMStore(path, flag='n', open=bsddb3.btopen) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) kwargs.setdefault('compressor', Zlib(1)) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_nbytes_stored(self): pass # not implemented @@ -1257,9 +1281,12 @@ def create_array(read_only=False, **kwargs): store = LMDBStore(path, buffers=True) except ImportError: # pragma: no cover raise SkipTest('lmdb not installed') + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) kwargs.setdefault('compressor', Zlib(1)) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_nbytes_stored(self): pass # not implemented @@ -1275,9 +1302,12 @@ def create_array(read_only=False, **kwargs): store = LMDBStore(path, buffers=False) except ImportError: # pragma: no cover raise SkipTest('lmdb not installed') + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) kwargs.setdefault('compressor', Zlib(1)) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_nbytes_stored(self): pass # not implemented @@ -1288,8 +1318,11 @@ class TestArrayWithNoCompressor(TestArray): def create_array(self, read_only=False, **kwargs): store = dict() kwargs.setdefault('compressor', None) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_hexdigest(self): # Check basic 1-D array @@ -1321,8 +1354,11 @@ def create_array(self, read_only=False, **kwargs): store = dict() compressor = BZ2(level=1) kwargs.setdefault('compressor', compressor) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_hexdigest(self): # Check basic 1-D array @@ -1354,8 +1390,11 @@ def create_array(self, read_only=False, **kwargs): store = dict() compressor = Blosc(cname='zstd', clevel=1, shuffle=1) kwargs.setdefault('compressor', compressor) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_hexdigest(self): # Check basic 1-D array @@ -1392,8 +1431,11 @@ def create_array(self, read_only=False, **kwargs): store = dict() compressor = LZMA(preset=1) kwargs.setdefault('compressor', compressor) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_hexdigest(self): # Check basic 1-D array @@ -1432,8 +1474,11 @@ def create_array(read_only=False, **kwargs): kwargs.setdefault('filters', filters) compressor = Zlib(1) kwargs.setdefault('compressor', compressor) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_attrs=cache_attrs, + cache_metadata=cache_metadata) def test_hexdigest(self): # Check basic 1-D array @@ -1555,8 +1600,11 @@ class TestArrayWithCustomMapping(TestArray): def create_array(read_only=False, **kwargs): store = CustomMapping() kwargs.setdefault('compressor', Zlib(1)) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_nbytes_stored(self): z = self.create_array(shape=1000, chunks=100) @@ -1571,12 +1619,14 @@ class TestArrayNoCache(TestArray): def create_array(read_only=False, **kwargs): store = dict() kwargs.setdefault('compressor', Zlib(level=1)) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only, cache_metadata=False, - cache_attrs=False) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def test_cache_metadata(self): - a1 = self.create_array(shape=100, chunks=10, dtype='i1') + a1 = self.create_array(shape=100, chunks=10, dtype='i1', cache_metadata=False) a2 = Array(a1.store, cache_metadata=True) eq(a1.shape, a2.shape) eq(a1.size, a2.size) @@ -1616,7 +1666,7 @@ def test_cache_metadata(self): eq(30, a2.nchunks) def test_cache_attrs(self): - a1 = self.create_array(shape=100, chunks=10, dtype='i1') + a1 = self.create_array(shape=100, chunks=10, dtype='i1', cache_attrs=False) a2 = Array(a1.store, cache_attrs=True) eq(a1.attrs.asdict(), a2.attrs.asdict()) diff --git a/zarr/tests/test_sync.py b/zarr/tests/test_sync.py index 0dee2ba397..9d967b1788 100644 --- a/zarr/tests/test_sync.py +++ b/zarr/tests/test_sync.py @@ -102,9 +102,12 @@ class TestArrayWithThreadSynchronizer(TestArray, MixinArraySyncTests): def create_array(self, read_only=False, **kwargs): store = dict() + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) return Array(store, synchronizer=ThreadSynchronizer(), - read_only=read_only) + read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs) def create_pool(self): pool = ThreadPool(cpu_count()) @@ -140,12 +143,14 @@ def create_array(self, read_only=False, **kwargs): path = tempfile.mkdtemp() atexit.register(atexit_rmtree, path) store = DirectoryStore(path) + cache_metadata = kwargs.pop('cache_metadata', False) + cache_attrs = kwargs.pop('cache_attrs', False) init_array(store, **kwargs) sync_path = tempfile.mkdtemp() atexit.register(atexit_rmtree, sync_path) synchronizer = ProcessSynchronizer(sync_path) - return Array(store, synchronizer=synchronizer, - read_only=read_only, cache_metadata=False) + return Array(store, synchronizer=synchronizer, read_only=read_only, + cache_metadata=cache_metadata, cache_attrs=cache_attrs) def create_pool(self): pool = ProcessPool(processes=cpu_count()) From 26d7366046cd56faf1e4be1cee748697f6241ef2 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 18:17:30 +0000 Subject: [PATCH 11/13] release notes for attribute caching --- docs/release.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/release.rst b/docs/release.rst index 8317026660..f43e28e5c1 100644 --- a/docs/release.rst +++ b/docs/release.rst @@ -127,6 +127,10 @@ Enhancements * **Added support for ``datetime64`` and ``timedelta64`` data types**; :issue:`85`, :issue:`215`. +* **Array and group attributes are now cached by default** to improve performance with + slow stores, e.g., stores accessing data via the network; :issue:`220`, :issue:`218`, + :issue:`204`. + Bug fixes ~~~~~~~~~ From 0f30f68c00b6edf9b3f9917359815d28a00f5155 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 18:31:42 +0000 Subject: [PATCH 12/13] implement Attributes.refresh --- docs/api/attrs.rst | 1 + zarr/attrs.py | 5 +++++ zarr/tests/test_attrs.py | 10 ++++++++++ 3 files changed, 16 insertions(+) diff --git a/docs/api/attrs.rst b/docs/api/attrs.rst index a22914b7d6..f95e63af3a 100644 --- a/docs/api/attrs.rst +++ b/docs/api/attrs.rst @@ -13,3 +13,4 @@ The Attributes class (``zarr.attrs``) .. automethod:: asdict .. automethod:: put .. automethod:: update + .. automethod:: refresh diff --git a/zarr/attrs.py b/zarr/attrs.py index c41c6e970b..9ea70e2f75 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -55,6 +55,11 @@ def asdict(self): self._cached_asdict = d return d + def refresh(self): + """Refresh cached attributes from the store.""" + if self.cache: + self._cached_asdict = self._get_nosync() + def __contains__(self, x): return x in self.asdict() diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index e869927b47..9f483772f4 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -168,6 +168,16 @@ def test_caching_on(self): eq(4, store.counter['__getitem__', 'attrs']) eq(4, store.counter['__setitem__', 'attrs']) + # test refresh() + store['attrs'] = json.dumps(dict(foo='xxx', bar=42)).encode('ascii') + eq(4, store.counter['__getitem__', 'attrs']) + a.refresh() + eq(5, store.counter['__getitem__', 'attrs']) + eq(a['foo'], 'xxx') + eq(5, store.counter['__getitem__', 'attrs']) + eq(a['bar'], 42) + eq(5, store.counter['__getitem__', 'attrs']) + def test_caching_off(self): # setup store From 45c0fd129e44a551d85306a09c24d01eb658cac3 Mon Sep 17 00:00:00 2001 From: Alistair Miles Date: Tue, 2 Jan 2018 18:39:17 +0000 Subject: [PATCH 13/13] fix tests after merge --- zarr/tests/test_core.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/zarr/tests/test_core.py b/zarr/tests/test_core.py index f52343f4c8..d89826a2a2 100644 --- a/zarr/tests/test_core.py +++ b/zarr/tests/test_core.py @@ -1692,5 +1692,8 @@ class TestArrayWithStoreCache(TestArray): def create_array(read_only=False, **kwargs): store = LRUStoreCache(dict(), max_size=None) kwargs.setdefault('compressor', Zlib(level=1)) + cache_metadata = kwargs.pop('cache_metadata', True) + cache_attrs = kwargs.pop('cache_attrs', True) init_array(store, **kwargs) - return Array(store, read_only=read_only) + return Array(store, read_only=read_only, cache_metadata=cache_metadata, + cache_attrs=cache_attrs)