Repository: zarr-developers/zarr-python
Issue: The read_only flag does not seem to be passed into the Array() constructor for in-memory array

It seems like in the create() function in zarr/creation.py does not pass the read_only argument into the Array() constructor.  It would be useful to have a read-only array that's not just file-based.

```
In [1]: import zarr

In [2]: a = zarr.array([1, 2, 3], read_only=True)
/.../site-packages/zarr/creation.py:155: UserWarning: ignoring keyword argument 'read_only'
  warn('ignoring keyword argument %r' % k)

In [3]: a[0] = 4

In [4]: print a[0]
4
```

I also can't figure out how to set the read_only flag after the array is being constructed.  In numpy you can do something like:

```
a = np.array([1, 2, 3])
a.flags.writeable = False
```

So after you've changed the fields, you can just set this to read_only to prevent people from accidentally changing the values.

Thanks,

Will




Changed files:
zarr/creation.py, zarr/tests/test_hierarchy.py, zarr/core.py, zarr/tests/test_creation.py, zarr/storage.py

diff --git a/zarr/core.py b/zarr/core.py
index 1901a0c1..8cafe30b 100644
--- a/zarr/core.py
+++ b/zarr/core.py
@@ -105,7 +105,7 @@ class Array(object):
             self._key_prefix = self._path + '/'
         else:
             self._key_prefix = ''
-        self._read_only = read_only
+        self._read_only = bool(read_only)
         self._synchronizer = synchronizer
         self._cache_metadata = cache_metadata
         self._is_view = False
@@ -216,6 +216,10 @@ class Array(object):
         """A boolean, True if modification operations are not permitted."""
         return self._read_only
 
+    @read_only.setter
+    def read_only(self, value):
+        self._read_only = bool(value)
+
     @property
     def chunk_store(self):
         """A MutableMapping providing the underlying storage for array chunks."""
diff --git a/zarr/creation.py b/zarr/creation.py
index 4dfc20c4..43622c78 100644
--- a/zarr/creation.py
+++ b/zarr/creation.py
@@ -17,7 +17,7 @@ from zarr.errors import err_contains_array, err_contains_group, \\
 def create(shape, chunks=None, 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, **kwargs):
+           cache_metadata=True, read_only=False, **kwargs):
     """Create an array.
 
     Parameters
@@ -53,6 +53,8 @@ def create(shape, chunks=None, 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).
+    read_only : bool, optional
+        True if array should be protected against modification.
 
     Returns
     -------
@@ -104,14 +106,13 @@ def create(shape, chunks=None, dtype=None, compressor='default',
     compressor, fill_value = _handle_kwargs(compressor, fill_value, kwargs)
 
     # initialize array metadata
-    init_array(store, shape=shape, chunks=chunks, dtype=dtype,
-               compressor=compressor, fill_value=fill_value, order=order,
-               overwrite=overwrite, path=path, chunk_store=chunk_store,
-               filters=filters)
+    init_array(store, shape=shape, chunks=chunks, dtype=dtype, compressor=compressor,
+               fill_value=fill_value, order=order, overwrite=overwrite, path=path,
+               chunk_store=chunk_store, filters=filters)
 
     # instantiate array
-    z = Array(store, path=path, chunk_store=chunk_store,
-              synchronizer=synchronizer, cache_metadata=cache_metadata)
+    z = Array(store, path=path, chunk_store=chunk_store, synchronizer=synchronizer,
+              cache_metadata=cache_metadata, read_only=read_only)
 
     return z
 
@@ -134,8 +135,10 @@ def _handle_kwargs(compressor, fill_value, kwargs):
         # 'compressor' overrides 'compression'
         if 'compression' in kwargs:
             warn("'compression' keyword argument overridden by 'compressor'")
+            del kwargs['compression']
         if 'compression_opts' in kwargs:
-            warn("ignoring keyword argument 'compression_opts'")
+            warn("'compression_opts' keyword argument overridden by 'compressor'")
+            del kwargs['compression_opts']
 
     elif 'compression' in kwargs:
         compression = kwargs.pop('compression')
@@ -316,12 +319,18 @@ def array(data, **kwargs):
     else:
         kwargs['chunks'] = kw_chunks
 
+    # pop read-only to apply after storing the data
+    read_only = kwargs.pop('read_only', False)
+
     # instantiate array
     z = create(**kwargs)
 
     # fill with data
     z[...] = data
 
+    # set read_only property afterwards
+    z.read_only = read_only
+
     return z
 
 
diff --git a/zarr/storage.py b/zarr/storage.py
index 302dc445..e691fdae 100644
--- a/zarr/storage.py
+++ b/zarr/storage.py
@@ -385,10 +385,10 @@ def ensure_bytes(s):
     if isinstance(s, np.ndarray):
         if PY2:  # pragma: py3 no cover
             # noinspection PyArgumentList
-            return s.tostring(order='Any')
+            return s.tostring(order='A')
         else:  # pragma: py2 no cover
             # noinspection PyArgumentList
-            return s.tobytes(order='Any')
+            return s.tobytes(order='A')
     if hasattr(s, 'tobytes'):
         return s.tobytes()
     if PY2 and hasattr(s, 'tostring'):  # pragma: py3 no cover
@@ -587,12 +587,14 @@ class DirectoryStore(MutableMapping):
     >>> store['foo'] = b'bar'
     >>> store['foo']
     b'bar'
-    >>> open('example_store/foo', 'rb').read()
+    >>> with open('example_store/foo', 'rb') as f:
+    ...     f.read()
     b'bar'
     >>> store['a/b/c'] = b'xxx'
     >>> store['a/b/c']
     b'xxx'
-    >>> open('example_store/a/b/c', 'rb').read()
+    >>> with open('example_store/a/b/c', 'rb') as f:
+    ...     f.read()
     b'xxx'
     >>> sorted(store.keys())
     ['a/b/c', 'foo']
@@ -801,9 +803,11 @@ class NestedDirectoryStore(DirectoryStore):
         >>> store['a/b/c'] = b'xxx'
         >>> store['a/b/c']
         b'xxx'
-        >>> open('example_nested_store/foo', 'rb').read()
+        >>> with open('example_nested_store/foo', 'rb') as f:
+        ...     f.read()
         b'bar'
-        >>> open('example_nested_store/a/b/c', 'rb').read()
+        >>> with open('example_nested_store/a/b/c', 'rb') as f:
+        ...     f.read()
         b'xxx'
 
     Chunk keys are handled in a special way, such that the '.' characters in the key are mapped to
@@ -815,9 +819,11 @@ class NestedDirectoryStore(DirectoryStore):
         >>> store['baz/2.1.12'] = b'zzz'
         >>> store['baz/2.1.12']
         b'zzz'
-        >>> open('example_nested_store/bar/0/0', 'rb').read()
+        >>> with open('example_nested_store/bar/0/0', 'rb') as f:
+        ...     f.read()
         b'yyy'
-        >>> open('example_nested_store/baz/2/1/12', 'rb').read()
+        >>> with open('example_nested_store/baz/2/1/12', 'rb') as f:
+        ...     f.read()
         b'zzz'
 
     Notes
diff --git a/zarr/tests/test_creation.py b/zarr/tests/test_creation.py
index e1590199..27beed58 100644
--- a/zarr/tests/test_creation.py
+++ b/zarr/tests/test_creation.py
@@ -3,6 +3,7 @@ from __future__ import absolute_import, print_function, division
 import tempfile
 import shutil
 import atexit
+import warnings
 
 
 import numpy as np
@@ -22,6 +23,11 @@ from zarr.codecs import Zlib
 from zarr.compat import PY2
 
 
+# needed for PY2/PY3 consistent behaviour
+warnings.resetwarnings()
+warnings.simplefilter('always')
+
+
 # something bcolz-like
 class MockBcolzArray(object):
 
@@ -424,6 +430,8 @@ def test_create():
 
 
 def test_compression_args():
+    warnings.resetwarnings()
+    warnings.simplefilter('always')
 
     z = create(100, compression='zlib', compression_opts=9)
     assert_is_instance(z, Array)
@@ -441,3 +449,39 @@ def test_compression_args():
     assert_is_instance(z, Array)
     eq('zlib', z.compressor.codec_id)
     eq(9, z.compressor.level)
+
+    warnings.resetwarnings()
+    warnings.simplefilter('error')
+    with assert_raises(UserWarning):
+        # 'compressor' overrides 'compression'
+        create(100, compressor=Zlib(9), compression='bz2', compression_opts=1)
+    with assert_raises(UserWarning):
+        # 'compressor' ignores 'compression_opts'
+        create(100, compressor=Zlib(9), compression_opts=1)
+    warnings.resetwarnings()
+    warnings.simplefilter('always')
+
+
+def test_create_read_only():
+    # https://github.com/alimanfoo/zarr/issues/151
+
+    # create an array initially read-only, then enable writing
+    z = create(100, read_only=True)
+    assert z.read_only
+    with assert_raises(PermissionError):
+        z[:] = 42
+    z.read_only = False
+    z[:] = 42
+    assert np.all(z[...] == 42)
+    z.read_only = True
+    with assert_raises(PermissionError):
+        z[:] = 0
+
+    # this is subtly different, but here we want to create an array with data, and then have it
+    # be read-only
+    a = np.arange(100)
+    z = array(a, read_only=True)
+    assert_array_equal(a, z[...])
+    assert z.read_only
+    with assert_raises(PermissionError):
+        z[:] = 42
diff --git a/zarr/tests/test_hierarchy.py b/zarr/tests/test_hierarchy.py
index 3e026c63..d554b09e 100644
--- a/zarr/tests/test_hierarchy.py
+++ b/zarr/tests/test_hierarchy.py
@@ -6,6 +6,7 @@ import atexit
 import shutil
 import os
 import pickle
+import warnings
 
 
 from nose.tools import assert_raises, eq_ as eq, assert_is, assert_true, \\
@@ -25,6 +26,11 @@ from zarr.util import InfoReporter
 from numcodecs import Zlib
 
 
+# needed for PY2/PY3 consistent behaviour
+warnings.resetwarnings()
+warnings.simplefilter('always')
+
+
 # noinspection PyStatementEffect
 class TestGroup(unittest.TestCase):
 
@@ -344,12 +350,21 @@ class TestGroup(unittest.TestCase):
         with assert_raises(KeyError):
             g.require_dataset('c/d', shape=100, chunks=10)
 
-        # h5py compatibility
+        # h5py compatibility, accept 'fillvalue'
         d = g.create_dataset('x', shape=100, chunks=10, fillvalue=42)
         eq(42, d.fill_value)
-        # ignore 'shuffle'
-        d = g.create_dataset('y', shape=100, chunks=10, shuffle=True)
+
+        # h5py compatibility, ignore 'shuffle'
+        warnings.resetwarnings()
+        warnings.simplefilter('always')
+        d = g.create_dataset('y1', shape=100, chunks=10, shuffle=True)
         assert not hasattr(d, 'shuffle')
+        warnings.resetwarnings()
+        warnings.simplefilter('error')
+        with assert_raises(UserWarning):
+            g.create_dataset('y2', shape=100, chunks=10, shuffle=True)
+        warnings.resetwarnings()
+        warnings.simplefilter('always')
 
         # read-only
         g = self.create_group(read_only=True)