Repository: zarr-developers/zarr-python

Issue:
NumPy integers not accepted in `chunks` argument to `zarr.zeros`
The following code works with zarr 2.3.2, but not with more-recently-released versions of zarr (tested on zarr 2.4.0 and zarr 2.6.1). It now gives a `TypeError` as a result of attempting to JSON serialize a NumPy integer.

```python
import numpy as np
import zarr

zarr.zeros((10,), chunks=(np.int64(2),))
```

With zarr 2.3.2, this creates a new array, as expected. With later versions of zarr, I get the following traceback (here with zarr 2.6.1):
```python
(seismic-zarr) mdickinson@mirzakhani temp % python bug.py             
Traceback (most recent call last):
  File "bug.py", line 4, in <module>
    zarr.zeros((10,), chunks=(np.int64(2),))
  File "/Users/mdickinson/.venvs/seismic-zarr/lib/python3.8/site-packages/zarr/creation.py", line 248, in zeros
    return create(shape=shape, fill_value=0, **kwargs)
  File "/Users/mdickinson/.venvs/seismic-zarr/lib/python3.8/site-packages/zarr/creation.py", line 121, in create
    init_array(store, shape=shape, chunks=chunks, dtype=dtype, compressor=compressor,
  File "/Users/mdickinson/.venvs/seismic-zarr/lib/python3.8/site-packages/zarr/storage.py", line 344, in init_array
    _init_array_metadata(store, shape=shape, chunks=chunks, dtype=dtype,
  File "/Users/mdickinson/.venvs/seismic-zarr/lib/python3.8/site-packages/zarr/storage.py", line 434, in _init_array_metadata
    store[key] = encode_array_metadata(meta)
  File "/Users/mdickinson/.venvs/seismic-zarr/lib/python3.8/site-packages/zarr/meta.py", line 75, in encode_array_metadata
    return json_dumps(meta)
  File "/Users/mdickinson/.venvs/seismic-zarr/lib/python3.8/site-packages/zarr/util.py", line 25, in json_dumps
    return json.dumps(o, indent=4, sort_keys=True, ensure_ascii=True,
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/__init__.py", line 234, in dumps
    return cls(
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/encoder.py", line 201, in encode
    chunks = list(chunks)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/encoder.py", line 431, in _iterencode
    yield from _iterencode_dict(o, _current_indent_level)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/encoder.py", line 405, in _iterencode_dict
    yield from chunks
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/encoder.py", line 325, in _iterencode_list
    yield from chunks
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/encoder.py", line 438, in _iterencode
    o = _default(o)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/3.8/lib/python3.8/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type int64 is not JSON serializable
```

This caused a regression in some other code using zarr: tests started failing after updating zarr.

Please provide the following:

* Value of ``zarr.__version__``: 2.6.1
* Value of ``numcodecs.__version__``: 0.7.3
* Version of Python interpreter: 3.8.7 (from MacPorts)
* Operating system (Linux/Windows/Mac): macOS 10.15.7 (Catalina)
* How Zarr was installed (e.g., "using pip into virtual environment", or "using conda"): using pip into a venv.

NumPy version: 1.20.0 (but I was also able to reproduce with NumPy 1.16.5).

Discussion:
pep8speaks: Hello @ericpre! Thanks for opening this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





* In the file [`zarr/util.py`](https://github.com/zarr-developers/zarr-python/blob/b726e0dd9672f348b57dbb400ba888baff98e68f/zarr/util.py):

> [Line 39:101](https://github.com/zarr-developers/zarr-python/blob/b726e0dd9672f348b57dbb400ba888baff98e68f/zarr/util.py#L39): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (118 > 100 characters)


joshmoore: Thanks, @ericpre. I've launched the GH workflows for testing. (Manual intervention is only needed on your first PR ;) )
codecov[bot]: # [Codecov](https://codecov.io/gh/zarr-developers/zarr-python/pull/933?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) Report
> Merging [#933](https://codecov.io/gh/zarr-developers/zarr-python/pull/933?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (b726e0d) into [master](https://codecov.io/gh/zarr-developers/zarr-python/commit/22ded1d656cb2034180cf8a3f23f82b51f79d8b4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) (22ded1d) will **decrease** coverage by `0.04%`.
> The diff coverage is `66.66%`.

```diff
@@            Coverage Diff             @@
##           master     #933      +/-   ##
==========================================
- Coverage   99.94%   99.90%   -0.05%     
==========================================
  Files          32       32              
  Lines       11225    11148      -77     
==========================================
- Hits        11219    11137      -82     
- Misses          6       11       +5     
```

| [Impacted Files](https://codecov.io/gh/zarr-developers/zarr-python/pull/933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | |
|---|---|---|
| [zarr/util.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci91dGlsLnB5) | `98.67% <44.44%> (-1.33%)` | :arrow_down: |
| [zarr/tests/test\_creation.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci90ZXN0cy90ZXN0X2NyZWF0aW9uLnB5) | `100.00% <100.00%> (ø)` | |
| [zarr/tests/test\_util.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci90ZXN0cy90ZXN0X3V0aWwucHk=) | `100.00% <100.00%> (ø)` | |
| [zarr/n5.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci9uNS5weQ==) | `100.00% <0.00%> (ø)` | |
| [zarr/core.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci9jb3JlLnB5) | `100.00% <0.00%> (ø)` | |
| [zarr/storage.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci9zdG9yYWdlLnB5) | `100.00% <0.00%> (ø)` | |
| [zarr/indexing.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci9pbmRleGluZy5weQ==) | `100.00% <0.00%> (ø)` | |
| [zarr/hierarchy.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci9oaWVyYXJjaHkucHk=) | `100.00% <0.00%> (ø)` | |
| [zarr/tests/conftest.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci90ZXN0cy9jb25mdGVzdC5weQ==) | `100.00% <0.00%> (ø)` | |
| [zarr/tests/test\_core.py](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-emFyci90ZXN0cy90ZXN0X2NvcmUucHk=) | `100.00% <0.00%> (ø)` | |
| ... and [9 more](https://codecov.io/gh/zarr-developers/zarr-python/pull/933/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | |

ericpre: @jakirkham, @joshmoore, does it look good to you now? :)
joshmoore: Now green after upgrading the main branch to match 2.11.
ericpre: @joshmoore, @jakirkham, would it be possible to get this PR reviewed again please? Thanks!
joshmoore: I've relaunched the build. I don't know of any other existing solution and as this is a pure addition, :+1: from my side. @jakirkham ?
ericpre: Ping @jakirkham.
jakirkham: Sorry for the very slow reply. This seems reasonable. Let's get it in.
joshmoore: Thanks, @jakirkham! @ericpre: sorry for the wait. Do you want to push a blurb to https://github.com/zarr-developers/zarr-python/blob/master/docs/release.rst, then we'll get this merged? The only heads up is that 2.11 releases are currently slightly in limbo while we work on https://github.com/zarr-developers/zarr-python/pull/1007 but that shouldn't take too long.
ericpre: > Thanks, @jakirkham! @ericpre: sorry for the wait. Do you want to push a blurb to https://github.com/zarr-developers/zarr-python/blob/master/docs/release.rst, then we'll get this merged?

Ok, done!
joshmoore: Thanks, @ericpre! :tada:

Patch:
diff --git a/docs/release.rst b/docs/release.rst
index 13c2f20d..7a5cf51d 100644
--- a/docs/release.rst
+++ b/docs/release.rst
@@ -14,6 +14,9 @@ Bug fixes
   value when empty chunks are read back in.
   By :user:`Vyas Ramasubramani <vyasr>`; :issue:`965`.
 
+* Add number encoder for ``json.dumps`` to support numpy intergers in
+  ``chunks`` arguments. By :user:`Eric Prestat <ericpre>` :issue:`697`.
+
 .. _release_2.11.1:
 
 2.11.1
diff --git a/zarr/util.py b/zarr/util.py
index 9f5f04f5..cc3bd503 100644
--- a/zarr/util.py
+++ b/zarr/util.py
@@ -33,10 +33,22 @@ object_codecs = {
 }
 
 
+class NumberEncoder(json.JSONEncoder):
+
+    def default(self, o):
+        # See json.JSONEncoder.default docstring for explanation
+        # This is necessary to encode numpy dtype
+        if isinstance(o, numbers.Integral):
+            return int(o)
+        if isinstance(o, numbers.Real):
+            return float(o)
+        return json.JSONEncoder.default(self, o)
+
+
 def json_dumps(o: Any) -> bytes:
     """Write JSON in a consistent, human-readable way."""
     return json.dumps(o, indent=4, sort_keys=True, ensure_ascii=True,
-                      separators=(',', ': ')).encode('ascii')
+                      separators=(',', ': '), cls=NumberEncoder).encode('ascii')
 
 
 def json_loads(s: str) -> Dict[str, Any]:


Test patch:
diff --git a/zarr/tests/test_creation.py b/zarr/tests/test_creation.py
index cfab4f79..ee99bc7c 100644
--- a/zarr/tests/test_creation.py
+++ b/zarr/tests/test_creation.py
@@ -714,3 +714,8 @@ def test_create_read_only(zarr_version):
     assert z.read_only
     with pytest.raises(PermissionError):
         z[:] = 42
+
+
+def test_json_dumps_chunks_numpy_dtype():
+    z = zeros((10,), chunks=(np.int64(2),))
+    assert np.all(z[...] == 0)
diff --git a/zarr/tests/test_util.py b/zarr/tests/test_util.py
index efe8e663..e9e1786a 100644
--- a/zarr/tests/test_util.py
+++ b/zarr/tests/test_util.py
@@ -4,8 +4,10 @@ from unittest import mock
 import numpy as np
 import pytest
 
-from zarr.util import (all_equal, flatten, guess_chunks, human_readable_size, info_html_report,
-                       info_text_report, is_total_slice, normalize_chunks,
+from zarr.core import Array
+from zarr.util import (all_equal, flatten, guess_chunks, human_readable_size,
+                       info_html_report, info_text_report, is_total_slice,
+                       json_dumps, normalize_chunks,
                        normalize_dimension_separator,
                        normalize_fill_value, normalize_order,
                        normalize_resize_args, normalize_shape, retry_call,
@@ -238,3 +240,11 @@ def test_all_equal():
     # all_equal(None, *) always returns False
     assert not all_equal(None, np.array([None, None]))
     assert not all_equal(None, np.array([None, 10]))
+
+
+def test_json_dumps_numpy_dtype():
+    assert json_dumps(np.int64(0)) == json_dumps(0)
+    assert json_dumps(np.float32(0)) == json_dumps(float(0))
+    # Check that we raise the error of the superclass for unsupported object
+    with pytest.raises(TypeError):
+        json_dumps(Array)