Repository: glotzerlab/signac
Pull request #180 (2019-04-29 17:15:28)

Fix/h5store setdefault
This pull request ensures that the return value of a call to `H5Store.setdefault` and `H5Group.setdefault` returns a reference to a `H5Group` object if the value is a dictionary.

For example:

```
foo = h5s.setdefault('foo', dict())
```
will return a dictionary before this patch.
Any further modifications of `foo` would not affect the file at all, but instead operate on a local instance of `dict`.

codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=h1) Report
> Merging [#180](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=desc) into [master](https://codecov.io/gh/glotzerlab/signac/commit/d0aaf648733861f0b64cdd93137dd643b6155c01?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/glotzerlab/signac/pull/180/graphs/tree.svg?width=650&token=FghbjTyzgM&height=150&src=pr)](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   64.99%   64.99%   +<.01%     
==========================================
  Files          37       37              
  Lines        5522     5528       +6     
==========================================
+ Hits         3589     3593       +4     
- Misses       1933     1935       +2
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [signac/core/h5store.py](https://codecov.io/gh/glotzerlab/signac/pull/180/diff?src=pr&el=tree#diff-c2lnbmFjL2NvcmUvaDVzdG9yZS5weQ==) | `91.33% <66.66%> (-0.55%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=footer). Last update [d0aaf64...4c91cd1](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=h1) Report
> Merging [#180](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=desc) into [master](https://codecov.io/gh/glotzerlab/signac/commit/d0aaf648733861f0b64cdd93137dd643b6155c01?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/glotzerlab/signac/pull/180/graphs/tree.svg?width=650&token=FghbjTyzgM&height=150&src=pr)](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   64.99%   64.99%   +<.01%     
==========================================
  Files          37       37              
  Lines        5522     5528       +6     
==========================================
+ Hits         3589     3593       +4     
- Misses       1933     1935       +2
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [signac/core/h5store.py](https://codecov.io/gh/glotzerlab/signac/pull/180/diff?src=pr&el=tree#diff-c2lnbmFjL2NvcmUvaDVzdG9yZS5weQ==) | `91.33% <66.66%> (-0.55%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=footer). Last update [d0aaf64...4c91cd1](https://codecov.io/gh/glotzerlab/signac/pull/180?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/changelog.txt b/changelog.txt
index 753203ee..9547534f 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -21,6 +21,7 @@ next
 
  - Add command line options ``--sp`` and ``--doc`` for ``signac find`` that allow users to display key-value pairs of the state point and document in combination with the job id (#97, #146).
  - Fix: Searches for whole numbers will match all numerically matching integers regardless of whether they are stored as decimals or whole numbers (#169).
+ - Fix: Passing an instance of dict to `H5Store.setdefault()` will return an instance of `H5Group` instead of a dict (#180).
 
 
 [1.0.0] -- 2019-02-28
diff --git a/signac/core/h5store.py b/signac/core/h5store.py
index a6cd545d..473daa99 100644
--- a/signac/core/h5store.py
+++ b/signac/core/h5store.py
@@ -210,6 +210,10 @@ class H5Group(MutableMapping):
         else:
             self.__setitem__(key, value)
 
+    def setdefault(self, key, value):
+        super(H5Group, self).setdefault(key, value)
+        return self.__getitem__(key)
+
     def __iter__(self):
         # The generator below should be refactored to use 'yield from'
         # once we drop Python 2.7 support.
@@ -433,6 +437,10 @@ class H5Store(MutableMapping):
         else:
             self.__delitem__(key)
 
+    def setdefault(self, key, value):
+        super(H5Store, self).setdefault(key, value)
+        return self.__getitem__(key)
+
     def __iter__(self):
         with _ensure_open(self):
             # The generator below should be refactored to use 'yield from'


Test patch:
diff --git a/tests/test_h5store.py b/tests/test_h5store.py
index 494eff40..c51b5c71 100644
--- a/tests/test_h5store.py
+++ b/tests/test_h5store.py
@@ -177,7 +177,11 @@ class H5StoreTest(BaseH5StoreTest):
         with self.open_h5store() as h5s:
             key = 'setgetexplicitnested'
             d = self.get_testdata()
-            h5s.setdefault('a', dict())
+            self.assertNotIn('a', h5s)
+            ret = h5s.setdefault('a', dict())
+            self.assertIn('a', h5s)
+            self.assertEqual(ret, h5s['a'])
+            self.assertTrue(hasattr(ret, '_store'))  # is an H5Group object
             child1 = h5s['a']
             child2 = h5s['a']
             self.assertEqual(child1, child2)