From 488828cb53a8874982404422e1b5e18d2d0a0cb1 Mon Sep 17 00:00:00 2001 From: Mattia Almansi Date: Mon, 4 Jul 2022 14:53:03 +0200 Subject: [PATCH 1/6] check type of attribute keys --- zarr/attrs.py | 5 +++++ zarr/tests/test_attrs.py | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/zarr/attrs.py b/zarr/attrs.py index 39683d45d9..031454c266 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -128,6 +128,11 @@ def put(self, d): self._write_op(self._put_nosync, dict(attributes=d)) def _put_nosync(self, d): + + d_to_check = d if self._version == 2 else d["attributes"] + if not all(isinstance(item, str) for item in d_to_check): + raise TypeError('attribute keys must be strings') + if self._version == 2: self.store[self.key] = json_dumps(d) if self.cache: diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index b8058d9d63..4d8e235154 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -268,3 +268,16 @@ def test_caching_off(self, zarr_version): get_cnt = 10 if zarr_version == 2 else 12 assert get_cnt == store.counter['__getitem__', attrs_key] assert 3 == store.counter['__setitem__', attrs_key] + + def test_wrong_keys(self, zarr_version): + store = _init_store(zarr_version) + a = self.init_attributes(store, zarr_version=zarr_version) + + with pytest.raises(TypeError, match="attribute keys must be strings"): + a[1] = "foo" + + with pytest.raises(TypeError, match="attribute keys must be strings"): + a.put({1: "foo"}) + + with pytest.raises(TypeError, match="attribute keys must be strings"): + a.update({1: "foo"}) From 4c07ba3fbc258daf37e35672fa1aa37c3665561e Mon Sep 17 00:00:00 2001 From: Mattia Almansi Date: Tue, 12 Jul 2022 17:20:12 +0200 Subject: [PATCH 2/6] introduce deprecation cycle --- zarr/attrs.py | 9 ++++++++- zarr/tests/test_attrs.py | 13 +++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index 031454c266..ccc69a450e 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -1,3 +1,4 @@ +import warnings from collections.abc import MutableMapping from zarr._storage.store import Store, StoreV3 @@ -131,7 +132,13 @@ def _put_nosync(self, d): d_to_check = d if self._version == 2 else d["attributes"] if not all(isinstance(item, str) for item in d_to_check): - raise TypeError('attribute keys must be strings') + warnings.warn( + "only attribute keys of type 'string' will be allowed in the future", + DeprecationWarning, + stacklevel=2 + ) + # TODO: Raise an error for non-string keys + # raise TypeError("attribute keys must be strings") if self._version == 2: self.store[self.key] = json_dumps(d) diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index 4d8e235154..72eeae0870 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -273,11 +273,16 @@ def test_wrong_keys(self, zarr_version): store = _init_store(zarr_version) a = self.init_attributes(store, zarr_version=zarr_version) - with pytest.raises(TypeError, match="attribute keys must be strings"): + warning_msg = "only attribute keys of type 'string' will be allowed in the future" + + with pytest.warns(DeprecationWarning, match=warning_msg): a[1] = "foo" - with pytest.raises(TypeError, match="attribute keys must be strings"): + with pytest.warns(DeprecationWarning, match=warning_msg): a.put({1: "foo"}) - with pytest.raises(TypeError, match="attribute keys must be strings"): - a.update({1: "foo"}) + with pytest.warns(DeprecationWarning, match=warning_msg): + with pytest.raises(TypeError): + # TODO: This error is why keys of type string will be deprecated. + # See: https://github.com/zarr-developers/zarr-python/issues/1037 + a.update({1: "foo"}) From f2acd99ffd93fa2b922e6da51b5ebb8bff06925e Mon Sep 17 00:00:00 2001 From: Mattia Almansi Date: Tue, 12 Jul 2022 17:25:01 +0200 Subject: [PATCH 3/6] fix typo --- zarr/tests/test_attrs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index 72eeae0870..03e7de4e6d 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -283,6 +283,6 @@ def test_wrong_keys(self, zarr_version): with pytest.warns(DeprecationWarning, match=warning_msg): with pytest.raises(TypeError): - # TODO: This error is why keys of type string will be deprecated. + # TODO: This error is why keys of type non-string will be deprecated. # See: https://github.com/zarr-developers/zarr-python/issues/1037 a.update({1: "foo"}) From f1584eb7b4332cd20e19166d8fdd2bcdbe0290f0 Mon Sep 17 00:00:00 2001 From: Mattia Almansi Date: Wed, 20 Jul 2022 11:29:23 +0200 Subject: [PATCH 4/6] stringify keys --- zarr/attrs.py | 13 +++++++++++-- zarr/tests/test_attrs.py | 5 +---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index ccc69a450e..701194b9fa 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -132,13 +132,22 @@ def _put_nosync(self, d): d_to_check = d if self._version == 2 else d["attributes"] if not all(isinstance(item, str) for item in d_to_check): + # TODO: Raise an error for non-string keys + # raise TypeError("attribute keys must be strings") warnings.warn( "only attribute keys of type 'string' will be allowed in the future", DeprecationWarning, stacklevel=2 ) - # TODO: Raise an error for non-string keys - # raise TypeError("attribute keys must be strings") + try: + # Stringify + d_to_check = {str(k): v for k, v in d_to_check.items()} + if self._version == 2: + d = d_to_check + else: + d["attributes"] = d_to_check + except TypeError as ex: + raise TypeError("attribute keys can not be stringified") from ex if self._version == 2: self.store[self.key] = json_dumps(d) diff --git a/zarr/tests/test_attrs.py b/zarr/tests/test_attrs.py index 03e7de4e6d..e4baf182b2 100644 --- a/zarr/tests/test_attrs.py +++ b/zarr/tests/test_attrs.py @@ -282,7 +282,4 @@ def test_wrong_keys(self, zarr_version): a.put({1: "foo"}) with pytest.warns(DeprecationWarning, match=warning_msg): - with pytest.raises(TypeError): - # TODO: This error is why keys of type non-string will be deprecated. - # See: https://github.com/zarr-developers/zarr-python/issues/1037 - a.update({1: "foo"}) + a.update({1: "foo"}) From df831c37a8785b0b123a4036fe96948e756ff6d6 Mon Sep 17 00:00:00 2001 From: Mattia Almansi Date: Wed, 20 Jul 2022 11:35:58 +0200 Subject: [PATCH 5/6] cleanup --- zarr/attrs.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index 701194b9fa..de7dcf3530 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -139,16 +139,17 @@ def _put_nosync(self, d): DeprecationWarning, stacklevel=2 ) + try: - # Stringify d_to_check = {str(k): v for k, v in d_to_check.items()} - if self._version == 2: - d = d_to_check - else: - d["attributes"] = d_to_check except TypeError as ex: raise TypeError("attribute keys can not be stringified") from ex + if self._version == 2: + d = d_to_check + else: + d["attributes"] = d_to_check + if self._version == 2: self.store[self.key] = json_dumps(d) if self.cache: From fa482ff15b1f6d910fc20902b349a2d0399f9aa2 Mon Sep 17 00:00:00 2001 From: Mattia Almansi Date: Fri, 22 Jul 2022 12:15:10 +0200 Subject: [PATCH 6/6] do not cover except --- zarr/attrs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zarr/attrs.py b/zarr/attrs.py index de7dcf3530..60dd7f1d79 100644 --- a/zarr/attrs.py +++ b/zarr/attrs.py @@ -142,7 +142,7 @@ def _put_nosync(self, d): try: d_to_check = {str(k): v for k, v in d_to_check.items()} - except TypeError as ex: + except TypeError as ex: # pragma: no cover raise TypeError("attribute keys can not be stringified") from ex if self._version == 2: