Repository: glotzerlab/signac
Pull request #117 (2019-02-08 15:18:09)

Expand and forward constructor arguments of H5Store to h5py.File.

<!-- Provide a general summary of your changes in the Title above -->

## Description
<!-- Describe your changes in detail -->
 * Provide users with more options to control how the underlying h5py.File
   is constructed.
 * Implemented unit tests that test opening in read- and write-only mode.

## Motivation and Context
It was not possible to control the open mode prior to this patch.
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->

## Types of Changes
<!-- Please select all items that apply either now or after creating the pull request: -->
- [ ] Documentation update
- [ ] Bug fix
- [x] New feature
- [ ] Breaking change<sup>1</sup>

This pull request is based on
- [ ] *master*, because it is a bug fix or an update to the documentation.
- [x] *develop*, because it introduces a new feature.

<sup>1</sup>The change breaks (or has the potential to break) existing functionality.

## Checklist:

- [x] The changes introduced by this pull request are covered by existing or newly introduced tests.

csadorf: @bdice Is this better?

Patch:
diff --git a/signac/core/h5store.py b/signac/core/h5store.py
index 7d7aed90..63f608e4 100644
--- a/signac/core/h5store.py
+++ b/signac/core/h5store.py
@@ -261,16 +261,28 @@ class H5Store(MutableMapping):
             assert 'foo' in h5s
             assert h5s.foo == 'bar'
             assert h5s['foo'] == 'bar'
+
+    :param filename:
+        The filename of the underlying HDF5-file.
+    :param mode:
+        The file open mode to use. Defaults to 'a' (append).
+    :param kwargs:
+        Additional keyword arguments to be forward to the h5py.File constructor
+        See documentation for :class:`h5py.File` for more information.
     """
-    __slots__ = ['_filename', '_file']
+    __slots__ = ['_filename', '_file', '_mode', '_kwargs']
 
     _thread_lock = RLock()
 
-    def __init__(self, filename):
+    def __init__(self, filename, mode=None, **kwargs):
         if not (isinstance(filename, six.string_types) and len(filename) > 0):
             raise ValueError('H5Store filename must be a non-empty string.')
+        if mode is None:
+            mode = 'a'
         self._filename = os.path.realpath(filename)
         self._file = None
+        self._mode = mode
+        self._kwargs = kwargs
 
     def __repr__(self):
         return "<{}(filename={})>".format(type(self).__name__, os.path.relpath(self._filename))
@@ -298,7 +310,7 @@ class H5Store(MutableMapping):
         import h5py
         self._thread_lock.acquire()
         try:
-            self._file = h5py.File(self._filename)
+            self._file = h5py.File(self._filename, mode=self._mode, **self._kwargs)
         except:  # noqa We need to release under **all** circumstances upon error!
             self._thread_lock.release()
             raise
@@ -323,6 +335,10 @@ class H5Store(MutableMapping):
         else:
             return self._file
 
+    @property
+    def mode(self):
+        return self._mode
+
     def flush(self):
         """Flush the underlying HDF5-file."""
         self._file.flush()


Test patch:
diff --git a/tests/test_h5store.py b/tests/test_h5store.py
index 54b6790f..cad204cc 100644
--- a/tests/test_h5store.py
+++ b/tests/test_h5store.py
@@ -57,20 +57,20 @@ class BaseH5StoreTest(unittest.TestCase):
         self._fn_store_other = os.path.join(self._tmp_dir.name, 'other_' + FN_STORE)
         self.addCleanup(self._tmp_dir.cleanup)
 
-    def get_h5store(self):
-        return H5Store(filename=self._fn_store)
+    def get_h5store(self, **kwargs):
+        return H5Store(filename=self._fn_store, **kwargs)
 
     @contextmanager
-    def open_h5store(self):
-        with self.get_h5store() as h5s:
+    def open_h5store(self, **kwargs):
+        with self.get_h5store(**kwargs) as h5s:
             yield h5s
 
-    def get_other_h5store(self):
-        return H5Store(filename=self._fn_store_other)
+    def get_other_h5store(self, **kwargs):
+        return H5Store(filename=self._fn_store_other, **kwargs)
 
     @contextmanager
-    def open_other_h5store(self):
-        with self.get_other_h5store() as h5s:
+    def open_other_h5store(self, **kwargs):
+        with self.get_other_h5store(**kwargs) as h5s:
             yield h5s
 
     def get_testdata(self, size=None):
@@ -87,6 +87,40 @@ class BaseH5StoreTest(unittest.TestCase):
             super(BaseH5StoreTest, self).assertEqual(a, b)
 
 
+class H5StoreOpenTests(BaseH5StoreTest):
+
+    def test_open(self):
+        h5s = self.get_h5store()
+        h5s.open()
+        h5s.close()
+
+    def test_open_read_only(self):
+        with self.open_h5store() as h5s:
+            h5s['foo'] = 'bar'
+
+        with self.open_h5store(mode='r') as h5s:
+            self.assertIn('foo', h5s)
+            self.assertEqual(h5s['foo'], 'bar')
+
+    def test_open_write_only(self):
+        with self.open_h5store(mode='w') as h5s:
+            h5s['foo'] = 'bar'
+            self.assertIn('foo', h5s)
+            self.assertEqual(h5s['foo'], 'bar')
+
+    def test_open_write_and_read_only(self):
+        with self.open_h5store(mode='w') as h5s_w:
+            with self.open_h5store(mode='r') as h5s_r:
+                self.assertNotIn('foo', h5s_r)
+                self.assertNotIn('foo', h5s_w)
+
+                h5s_w['foo'] = 'bar'
+                self.assertIn('foo', h5s_r)
+                self.assertEqual(h5s_r['foo'], 'bar')
+                self.assertIn('foo', h5s_w)
+                self.assertEqual(h5s_r['foo'], 'bar')
+
+
 class H5StoreTest(BaseH5StoreTest):
 
     valid_types = {
@@ -483,8 +517,8 @@ class H5StoreClosedTest(H5StoreTest):
     }
 
     @contextmanager
-    def open_h5store(self):
-        yield self.get_h5store()
+    def open_h5store(self, **kwargs):
+        yield self.get_h5store(**kwargs)
 
 
 class H5StoreNestedDataClosedTest(H5StoreNestedDataTest, H5StoreClosedTest):