TST+BF: Expand tests and fix bugs

Author: moloney

nibabel/metasum.py

@@ -125,7 +125,7 @@ def get_mask(self, value):
             return res
         return self._val_bitarrs[value].copy()
 
-    def num_indices(self, value, mask=None):
+    def count(self, value, mask=None):
         '''Number of indices for the given `value`'''
         if mask is not None:
             if len(mask) != self.n_input:
@@ -136,15 +136,15 @@ def num_indices(self, value, mask=None):
             if mask is None:
                 return self._n_input
             return mask.count()
-        unique_idx = self._unique_vals.get(_NoValue)
+        unique_idx = self._unique_vals.get(value, _NoValue)
         if unique_idx is not _NoValue:
             if mask is not None:
                 if mask[unique_idx]:
                     return 1
                 return 0
             return 1
         if mask is not None:
-            return (self._val_bitarrs[value] & mask).count
+            return (self._val_bitarrs[value] & mask).count()
         return self._val_bitarrs[value].count()
 
     def get_value(self, idx):
@@ -169,14 +169,14 @@ def to_list(self):
 
     def extend(self, values):
         '''Add more values to the end of any existing ones'''
-        curr_size = self._n_input
+        init_size = self._n_input
         if isinstance(values, ValueIndices):
             other_is_vi = True
             other_size = values._n_input
         else:
             other_is_vi = False
             other_size = len(values)
-        final_size = curr_size + other_size
+        final_size = init_size + other_size
         for ba in self._val_bitarrs.values():
             ba.extend(zeros(other_size))
         if other_is_vi:
@@ -185,7 +185,7 @@ def extend(self, values):
                     self._extend_const(values)
                     return
                 else:
-                    self._rm_const()
+                    self._rm_const(final_size)
             elif values._const_val is not _NoValue:
                 cval = values._const_val
                 other_unique = {}
@@ -199,40 +199,49 @@ def extend(self, values):
                 other_unique = values._unique_vals
                 other_bitarrs = values._val_bitarrs
             for val, other_idx in other_unique.items():
-                self._ingest_single(val, final_size, curr_size, other_idx)
+                self._ingest_single(val, final_size, init_size, other_idx)
             for val, other_ba in other_bitarrs.items():
                 curr_ba = self._val_bitarrs.get(val)
                 if curr_ba is None:
                     curr_idx = self._unique_vals.get(val)
                     if curr_idx is None:
-                        if curr_size == 0:
+                        if init_size == 0:
                             new_ba = other_ba.copy()
                         else:
-                            new_ba = zeros(curr_size)
+                            new_ba = zeros(init_size)
                             new_ba.extend(other_ba)
                     else:
-                        new_ba = zeros(curr_size)
+                        new_ba = zeros(init_size)
                         new_ba[curr_idx] = True
                         new_ba.extend(other_ba)
                         del self._unique_vals[val]
                     self._val_bitarrs[val] = new_ba
                 else:
-                    curr_ba[curr_size:] |= other_ba
+                    curr_ba[init_size:] |= other_ba
+                self._n_input += other_ba.count()
         else:
             for other_idx, val in enumerate(values):
-                self._ingest_single(val, final_size, curr_size, other_idx)
-        self._n_input = final_size
+                self._ingest_single(val, final_size, init_size, other_idx)
+        assert self._n_input == final_size
 
     def append(self, value):
         '''Append another value as input'''
         if self._const_val == value:
             self._n_input += 1
             return
         elif self._const_val is not _NoValue:
-            self._rm_const()
+            self._rm_const(self._n_input + 1)
+            self._unique_vals[value] = self._n_input
+            self._n_input += 1
+            return
+        if self._n_input == 0:
+            self._const_val = value
+            self._n_input += 1
+            return
         curr_size = self._n_input
         found = False
         for val, bitarr in self._val_bitarrs.items():
+            assert len(bitarr) == self._n_input
             if val == value:
                 found = True
                 bitarr.append(True)
@@ -318,7 +327,7 @@ def get_block_size(self):
         if rem != 0:
             return None
         for val in self.values():
-            if self.num_indices(val) != block_size:
+            if self.count(val) != block_size:
                 return None
         return block_size
 
@@ -335,32 +344,43 @@ def _extract_indices(self, ba):
             except ValueError:
                 return
             yield curr_idx
-            start = curr_idx
+            start = curr_idx + 1
 
-    def _ingest_single(self, val, final_size, curr_size, other_idx):
+    def _ingest_single(self, val, final_size, init_size, other_idx):
         '''Helper to ingest single value from another collection'''
+        if val == self._const_val:
+            self._n_input += 1
+            return
+        elif self._const_val is not _NoValue:
+            self._rm_const(final_size)
+        if self._n_input == 0:
+            self._const_val = val
+            self._n_input += 1
+            return
+
         curr_ba = self._val_bitarrs.get(val)
         if curr_ba is None:
             curr_idx = self._unique_vals.get(val)
             if curr_idx is None:
-                self._unique_vals[val] = curr_size + other_idx
+                self._unique_vals[val] = init_size + other_idx
             else:
                 new_ba = zeros(final_size)
                 new_ba[curr_idx] = True
-                new_ba[curr_size + other_idx] = True
+                new_ba[init_size + other_idx] = True
                 self._val_bitarrs[val] = new_ba
                 del self._unique_vals[val]
         else:
-            curr_ba[curr_size + other_idx] = True
+            curr_ba[init_size + other_idx] = True
+        self._n_input += 1
 
-    def _rm_const(self):
+    def _rm_const(self, final_size):
         assert self._const_val is not _NoValue
         if self._n_input == 1:
             self._unique_vals[self._const_val] = 0
         else:
-            self._val_bitarrs[self._const_val] = bitarray(self._n_input)
-            self._val_bitarrs[self._const_val].setall(1)
-        self._const_val == _NoValue
+            self._val_bitarrs[self._const_val] = zeros(final_size)
+            self._val_bitarrs[self._const_val][:self._n_input] = True
+        self._const_val = _NoValue
 
     def _extend_const(self, other):
         if self._const_val != other._const_val:

nibabel/tests/test_metasum.py

@@ -13,9 +13,16 @@
 
 
 @pytest.mark.parametrize("in_list", vidx_test_patterns)
-def test_value_indices_rt(in_list):
+def test_value_indices_basics(in_list):
     '''Test we can roundtrip list -> ValueIndices -> list'''
     vidx = ValueIndices(in_list)
+    assert vidx.n_input == len(in_list)
+    assert len(vidx) == len(set(in_list))
+    assert sorted(vidx.values()) == sorted(list(set(in_list)))
+    for val in vidx.values():
+        assert vidx.count(val) == in_list.count(val)
+        for in_idx in vidx[val]:
+            assert in_list[in_idx] == val
     out_list = vidx.to_list()
     assert in_list == out_list
 
@@ -40,22 +47,32 @@ def test_value_indices_append_extend(in_list):
         assert vidx.to_list() == in_list + in_list
 
 
-metasum_test_dicts = (({'key1': 0, 'key2': 'a', 'key3': 3.0},
-                       {'key1': 2, 'key2': 'c', 'key3': 1.0},
-                       {'key1': 1, 'key2': 'b', 'key3': 2.0},
+metasum_test_dicts = (({'u1': 0, 'u2': 'a', 'u3': 3.0, 'c1': True, 'r1': 5},
+                       {'u1': 2, 'u2': 'c', 'u3': 1.0, 'c1': True, 'r1': 5},
+                       {'u1': 1, 'u2': 'b', 'u3': 2.0, 'c1': True, 'r1': 7},
                        ),
-                      ({'key1': 0, 'key2': 'a', 'key3': 3.0},
-                       {'key1': 2, 'key2': 'c'},
-                       {'key1': 1, 'key2': 'b', 'key3': 2.0},
+                      ({'u1': 0, 'u2': 'a', 'u3': 3.0, 'c1': True, 'r1': 5},
+                       {'u1': 2, 'u2': 'c', 'c1': True, 'r1': 5},
+                       {'u1': 1, 'u2': 'b', 'u3': 2.0, 'c1': True},
                        ),
                       )
 
 
 @pytest.mark.parametrize("in_dicts", metasum_test_dicts)
-def test_meta_summary_rt(in_dicts):
+def test_meta_summary_basics(in_dicts):
     msum = MetaSummary()
+    all_keys = set()
     for in_dict in in_dicts:
         msum.append(in_dict)
+        for key in in_dict.keys():
+            all_keys.add(key)
+    assert all_keys == set(msum.keys())
+    for key in msum.const_keys():
+        assert key.startswith('c')
+    for key in msum.unique_keys():
+        assert key.startswith('u')
+    for key in msum.repeating_keys():
+        assert key.startswith('r')
     for in_idx in range(len(in_dicts)):
         out_dict = msum.get_meta(in_idx)
         assert out_dict == in_dicts[in_idx]