From 96d11834ff3e2f618614d937ede7c07366b6b210 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Thu, 13 Oct 2022 18:37:13 +0100 Subject: [PATCH 01/17] zipstore delitems via override --- zarr/storage.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/zarr/storage.py b/zarr/storage.py index f5459990ba..0c97102164 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1779,7 +1779,13 @@ def __setitem__(self, key, value): self.zf.writestr(keyinfo, value) def __delitem__(self, key): - raise NotImplementedError + # raise NotImplementedError + try: + value = self[key] + except KeyError: + raise KeyError("Can not delete a nonexistent key") + + self[key] = b"" def __eq__(self, other): return ( From a83bff2b06ceda1449939bce5048d0f7caaec38e Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Fri, 14 Oct 2022 14:19:22 +0100 Subject: [PATCH 02/17] test-for delete and pop item --- zarr/storage.py | 3 +++ zarr/tests/test_storage.py | 16 ++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 0c97102164..3a8823b4b7 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1787,6 +1787,9 @@ def __delitem__(self, key): self[key] = b"" + def pop(self, key): + del self[key] + def __eq__(self, other): return ( isinstance(other, ZipStore) and diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index 39d4b5988d..0db032c789 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -1802,12 +1802,24 @@ def test_context_manager(self): store[self.root + 'baz'] = b'qux' assert 2 == len(store) + def test_delitem(self): + store = self.create_store() + store[self.root + 'foo'] = b'bar' + + del store[self.root + 'foo'] + + assert store[self.root + 'foo'] == b'' + def test_pop(self): # override because not implemented store = self.create_store() store[self.root + 'foo'] = b'bar' - with pytest.raises(NotImplementedError): - store.pop(self.root + 'foo') + + store.pop(self.root + 'foo') + + assert store[self.root + 'foo'] == b"" + # with pytest.raises(NotImplementedError): + # store.pop(self.root + 'foo') def test_popitem(self): # override because not implemented From 0144f953e99ec73f4d03a7e6825ea48578e9e600 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Fri, 14 Oct 2022 16:52:50 +0100 Subject: [PATCH 03/17] i-am-emmanuel-zip-del_item --- zarr/storage.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 3a8823b4b7..8e6bb799d4 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1780,16 +1780,13 @@ def __setitem__(self, key, value): def __delitem__(self, key): # raise NotImplementedError - try: - value = self[key] - except KeyError: + if key not in self: raise KeyError("Can not delete a nonexistent key") self[key] = b"" def pop(self, key): - del self[key] - + del self def __eq__(self, other): return ( isinstance(other, ZipStore) and From 16fb300582af500455db07860f935e63664adfd0 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Thu, 20 Oct 2022 17:44:50 +0100 Subject: [PATCH 04/17] modified delete_item stored item --- zarr/storage.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 8e6bb799d4..b181780a03 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1780,13 +1780,31 @@ def __setitem__(self, key, value): def __delitem__(self, key): # raise NotImplementedError - if key not in self: + value = b"" + try: + with self.mutex: + self.zf.getinfo(key) + keyinfo = zipfile.ZipInfo(filename=key, + date_time=time.localtime(time.time())[:6]) + keyinfo.compress_type = self.compression + if keyinfo.filename[-1] == os.sep: + keyinfo.external_attr = 0o40775 << 16 # drwxrwxr-x + keyinfo.external_attr |= 0x10 # MS-DOS directory flag + else: + keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + + self.zf.writestr(keyinfo, value) + except KeyError: raise KeyError("Can not delete a nonexistent key") - self[key] = b"" + # if key not in self: + # + # with self.mutex: + # self[key] = b"" def pop(self, key): - del self + del self[key] + def __eq__(self, other): return ( isinstance(other, ZipStore) and From bf183970f26ea0d5fbbfdef9f636fca4c606ddc5 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Thu, 20 Oct 2022 18:11:30 +0100 Subject: [PATCH 05/17] modified delete_item stored item --- zarr/storage.py | 3 --- zarr/tests/test_storage.py | 31 ++++++++++++++++--------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index b181780a03..2ff5729879 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1801,9 +1801,6 @@ def __delitem__(self, key): # # with self.mutex: # self[key] = b"" - - def pop(self, key): - del self[key] def __eq__(self, other): return ( diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index 0db032c789..78bfedd64b 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -192,6 +192,14 @@ def test_pop(self): store.close() + def test_delitem(self): + store = self.create_store() + store[self.root + 'foo'] = b'bar' + + del store[self.root + 'foo'] + + assert store[self.root + 'foo'] == b'' + def test_popitem(self): store = self.create_store() store[self.root + 'foo'] = b'bar' @@ -1802,24 +1810,17 @@ def test_context_manager(self): store[self.root + 'baz'] = b'qux' assert 2 == len(store) - def test_delitem(self): - store = self.create_store() - store[self.root + 'foo'] = b'bar' - - del store[self.root + 'foo'] - - assert store[self.root + 'foo'] == b'' - def test_pop(self): - # override because not implemented - store = self.create_store() - store[self.root + 'foo'] = b'bar' + # def test_pop(self): + # # override because not implemented + # store = self.create_store() + # store[self.root + 'foo'] = b'bar' - store.pop(self.root + 'foo') + # store.pop(self.root + 'foo') - assert store[self.root + 'foo'] == b"" - # with pytest.raises(NotImplementedError): - # store.pop(self.root + 'foo') + # assert store[self.root + 'foo'] == b"" + # # with pytest.raises(NotImplementedError): + # # store.pop(self.root + 'foo') def test_popitem(self): # override because not implemented From d2d49de923fc0a15f23c2691bb2ccdad0b31ba5d Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Thu, 20 Oct 2022 18:14:14 +0100 Subject: [PATCH 06/17] modified delete_item stored item --- zarr/storage.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 2ff5729879..7874aa3bf8 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1795,12 +1795,7 @@ def __delitem__(self, key): self.zf.writestr(keyinfo, value) except KeyError: - raise KeyError("Can not delete a nonexistent key") - - # if key not in self: - # - # with self.mutex: - # self[key] = b"" + raise KeyError("Cannot delete a non-existent key") def __eq__(self, other): return ( From 60f9f61b50a889d49598d87ec66ffe69872bf125 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Thu, 20 Oct 2022 19:02:27 +0100 Subject: [PATCH 07/17] modified delete_item stored item --- zarr/storage.py | 5 +++-- zarr/tests/test_storage.py | 17 +++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 7874aa3bf8..74372035b7 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1784,8 +1784,8 @@ def __delitem__(self, key): try: with self.mutex: self.zf.getinfo(key) - keyinfo = zipfile.ZipInfo(filename=key, - date_time=time.localtime(time.time())[:6]) + keyinfo = zipfile.ZipInfo(filename=key, + date_time=time.localtime(time.time())[:6]) keyinfo.compress_type = self.compression if keyinfo.filename[-1] == os.sep: keyinfo.external_attr = 0o40775 << 16 # drwxrwxr-x @@ -1823,6 +1823,7 @@ def __contains__(self, key): try: with self.mutex: self.zf.getinfo(key) + except KeyError: return False else: diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index 78bfedd64b..1d8ddd87aa 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -1808,19 +1808,16 @@ def test_context_manager(self): with self.create_store() as store: store[self.root + 'foo'] = b'bar' store[self.root + 'baz'] = b'qux' - assert 2 == len(store) - + assert 2 == len(store) - # def test_pop(self): - # # override because not implemented - # store = self.create_store() - # store[self.root + 'foo'] = b'bar' + def test_pop(self): + # override because not implemented + store = self.create_store() + store[self.root + 'foo'] = b'bar' - # store.pop(self.root + 'foo') + store.pop(self.root + 'foo') - # assert store[self.root + 'foo'] == b"" - # # with pytest.raises(NotImplementedError): - # # store.pop(self.root + 'foo') + assert store[self.root + 'foo'] == b"" def test_popitem(self): # override because not implemented From 6366a2c5ab668793c44cbb1e0125677c187c02f9 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Thu, 20 Oct 2022 19:33:00 +0100 Subject: [PATCH 08/17] modified delete_item stored_item --- zarr/storage.py | 32 +++++++++++++++++--------------- zarr/tests/test_storage.py | 2 +- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 74372035b7..382dd2b745 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1779,24 +1779,26 @@ def __setitem__(self, key, value): self.zf.writestr(keyinfo, value) def __delitem__(self, key): - # raise NotImplementedError + value = b"" - try: - with self.mutex: + + with self.mutex: + try: self.zf.getinfo(key) - keyinfo = zipfile.ZipInfo(filename=key, - date_time=time.localtime(time.time())[:6]) - keyinfo.compress_type = self.compression - if keyinfo.filename[-1] == os.sep: - keyinfo.external_attr = 0o40775 << 16 # drwxrwxr-x - keyinfo.external_attr |= 0x10 # MS-DOS directory flag - else: - keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + except KeyError: + raise KeyError("Cannot delete a non-existent key") + keyinfo = zipfile.ZipInfo(filename=key, + date_time=time.localtime(time.time())[:6]) + keyinfo.compress_type = self.compression + if keyinfo.filename[-1] == os.sep: + keyinfo.external_attr = 0o40775 << 16 # drwxrwxr-x + keyinfo.external_attr |= 0x10 # MS-DOS directory flag + else: + keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + + self.zf.writestr(keyinfo, value) + - self.zf.writestr(keyinfo, value) - except KeyError: - raise KeyError("Cannot delete a non-existent key") - def __eq__(self, other): return ( isinstance(other, ZipStore) and diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index 1d8ddd87aa..f67d5bc4b3 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -1800,7 +1800,7 @@ def test_flush(self): store.flush() assert store[self.root + 'foo'] == b'bar' store.close() - + store = self.ZipStoreClass('data/store.zip', mode='r') store.flush() # no-op From a65d3831d8c457c47bc8c45a272073b72043d07b Mon Sep 17 00:00:00 2001 From: jakirkham Date: Thu, 20 Oct 2022 12:59:09 -0700 Subject: [PATCH 09/17] Cleanup blank spaces --- zarr/tests/test_storage.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index f67d5bc4b3..36c059ddaf 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -1800,7 +1800,6 @@ def test_flush(self): store.flush() assert store[self.root + 'foo'] == b'bar' store.close() - store = self.ZipStoreClass('data/store.zip', mode='r') store.flush() # no-op @@ -1808,7 +1807,7 @@ def test_context_manager(self): with self.create_store() as store: store[self.root + 'foo'] = b'bar' store[self.root + 'baz'] = b'qux' - assert 2 == len(store) + assert 2 == len(store) def test_pop(self): # override because not implemented From a41af487039e547306223e99d11c1d9afc7d095e Mon Sep 17 00:00:00 2001 From: jakirkham Date: Thu, 20 Oct 2022 13:04:20 -0700 Subject: [PATCH 10/17] Fix lints --- zarr/storage.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 382dd2b745..e659af084c 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1779,16 +1779,16 @@ def __setitem__(self, key, value): self.zf.writestr(keyinfo, value) def __delitem__(self, key): - value = b"" - with self.mutex: try: self.zf.getinfo(key) except KeyError: raise KeyError("Cannot delete a non-existent key") - keyinfo = zipfile.ZipInfo(filename=key, - date_time=time.localtime(time.time())[:6]) + keyinfo = zipfile.ZipInfo( + filename=key, + date_time=time.localtime(time.time())[:6] + ) keyinfo.compress_type = self.compression if keyinfo.filename[-1] == os.sep: keyinfo.external_attr = 0o40775 << 16 # drwxrwxr-x @@ -1797,7 +1797,6 @@ def __delitem__(self, key): keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- self.zf.writestr(keyinfo, value) - def __eq__(self, other): return ( From ab5210617e8f1edcdac4d6a90d15d2e7961d7b12 Mon Sep 17 00:00:00 2001 From: jakirkham Date: Thu, 20 Oct 2022 13:05:24 -0700 Subject: [PATCH 11/17] Fix one more lint --- zarr/storage.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zarr/storage.py b/zarr/storage.py index e659af084c..8b574ececf 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1786,7 +1786,7 @@ def __delitem__(self, key): except KeyError: raise KeyError("Cannot delete a non-existent key") keyinfo = zipfile.ZipInfo( - filename=key, + filename=key, date_time=time.localtime(time.time())[:6] ) keyinfo.compress_type = self.compression From 425a744bf8d85329c2f7e2670111290f70da61bd Mon Sep 17 00:00:00 2001 From: jakirkham Date: Thu, 20 Oct 2022 13:07:32 -0700 Subject: [PATCH 12/17] Inline `value` --- zarr/storage.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 8b574ececf..85ce584efd 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1779,7 +1779,6 @@ def __setitem__(self, key, value): self.zf.writestr(keyinfo, value) def __delitem__(self, key): - value = b"" with self.mutex: try: self.zf.getinfo(key) @@ -1796,7 +1795,7 @@ def __delitem__(self, key): else: keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- - self.zf.writestr(keyinfo, value) + self.zf.writestr(keyinfo, b"") def __eq__(self, other): return ( From 57ce0612eae76254e7935fe53a1063f4277273ec Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Tue, 25 Oct 2022 12:37:37 +0100 Subject: [PATCH 13/17] Modified zip_store del_item --- zarr/tests/test_storage.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index f67d5bc4b3..3c6f0fb8ac 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -9,6 +9,7 @@ import tempfile from contextlib import contextmanager from pickle import PicklingError +from xxlimited import foo from zipfile import ZipFile import numpy as np @@ -194,11 +195,16 @@ def test_pop(self): def test_delitem(self): store = self.create_store() - store[self.root + 'foo'] = b'bar' - - del store[self.root + 'foo'] + foo = self.root + 'foo' + store[foo] = b'bar' - assert store[self.root + 'foo'] == b'' + del store[foo] + + # if foo in store: + if isinstance(store, ZipStore): + assert store[foo] == b'' + else: + assert foo not in store def test_popitem(self): store = self.create_store() From 040da5fafc947b0f485cf55de790322fc7a4fe31 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Tue, 25 Oct 2022 18:29:30 +0100 Subject: [PATCH 14/17] Modified Zip_del_item --- zarr/tests/test_storage.py | 1 - 1 file changed, 1 deletion(-) diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index a9e95bc33b..f76fbd1b90 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -9,7 +9,6 @@ import tempfile from contextlib import contextmanager from pickle import PicklingError -from xxlimited import foo from zipfile import ZipFile import numpy as np From 5f8b815e95edb6746af1a066325824eec720c731 Mon Sep 17 00:00:00 2001 From: jakirkham Date: Tue, 25 Oct 2022 12:46:03 -0700 Subject: [PATCH 15/17] Dropping extra spaces (for linter) --- zarr/tests/test_storage.py | 1 - 1 file changed, 1 deletion(-) diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index f76fbd1b90..a46fcd00ec 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -198,7 +198,6 @@ def test_delitem(self): store[foo] = b'bar' del store[foo] - # if foo in store: if isinstance(store, ZipStore): assert store[foo] == b'' From 489390a5c9397a121bc086c21f872c68535b7341 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Wed, 26 Oct 2022 13:54:11 +0100 Subject: [PATCH 16/17] Remodified zip_del_item --- zarr/storage.py | 7 ++++++- zarr/tests/test_storage.py | 11 +++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 85ce584efd..ff249a01b5 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1757,7 +1757,12 @@ def __exit__(self, *args): def __getitem__(self, key): with self.mutex: with self.zf.open(key) as f: # will raise KeyError - return f.read() + data = f.read() + + if data: + return data + else: + raise KeyError("Key not found") def __setitem__(self, key, value): if self.mode == 'r': diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index f76fbd1b90..c4a354b975 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -198,12 +198,15 @@ def test_delitem(self): store[foo] = b'bar' del store[foo] + + with pytest.raises(KeyError): + store[foo] # if foo in store: - if isinstance(store, ZipStore): - assert store[foo] == b'' - else: - assert foo not in store + # if isinstance(store, ZipStore): + # assert store[foo] == b'' + # else: + # assert foo not in store def test_popitem(self): store = self.create_store() From dcce2c33061225259a8bce55ef654668ce2333a2 Mon Sep 17 00:00:00 2001 From: hemakulate28 Date: Wed, 2 Nov 2022 18:41:17 +0100 Subject: [PATCH 17/17] refactor __contains__ and keylist to reflect deleted key in zipstore --- zarr/storage.py | 8 ++++++-- zarr/tests/test_storage.py | 1 - 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/zarr/storage.py b/zarr/storage.py index 75468c1f6b..f16fd84f98 100644 --- a/zarr/storage.py +++ b/zarr/storage.py @@ -1810,7 +1810,8 @@ def __eq__(self, other): def keylist(self): with self.mutex: - return sorted(self.zf.namelist()) + namelist = [key for key in self.zf.namelist() if self.zf.getinfo(key) != b""] + return sorted(namelist) def keys(self): yield from self.keylist() @@ -1824,7 +1825,10 @@ def __len__(self): def __contains__(self, key): try: with self.mutex: - self.zf.getinfo(key) + value = self.zf.getinfo(key) + + if value == b"": + raise KeyError except KeyError: return False diff --git a/zarr/tests/test_storage.py b/zarr/tests/test_storage.py index c4a354b975..9d4a4fbaeb 100644 --- a/zarr/tests/test_storage.py +++ b/zarr/tests/test_storage.py @@ -201,7 +201,6 @@ def test_delitem(self): with pytest.raises(KeyError): store[foo] - # if foo in store: # if isinstance(store, ZipStore): # assert store[foo] == b''