Merge pull request #423 from matthew-brett/refactor-mroi

MRG: refactor mroi ROI selection

* rename variables in loops for clarity (see PR #422 for discussion);
* fix and test empty ROI selection.

nipy/labs/spatial_models/mroi.py

@@ -69,6 +69,9 @@ def __init__(self, domain, label, id=None):
           should access ROI through their id to avoid hazardous manipulations.
 
         """
+        self._init(domain, label, id)
+
+    def _init(self, domain, label, id=None):
         # check that label size is consistent with domain
         if np.size(label) != domain.size:
             raise ValueError('inconsistent labels and domains specification')
@@ -686,7 +689,7 @@ def select_roi(self, id_list):
         """
         # handle the case of an empty selection
         if len(id_list) == 0:
-            self = SubDomains(self.domain, -np.ones(self.label.size))
+            self._init(self.domain, -np.ones(self.label.size))
             return
         # convert id to indices
         id_list_pos = np.ravel([self.select_id(k) for k in id_list])
@@ -699,19 +702,19 @@ def select_roi(self, id_list):
 
         # set new features
         # (it's ok to do that after labels and id modification since we are
-        # poping out the former features and use the former id indices)
-        for fid in list(self.features):
-            f = self.remove_feature(fid)
-            sf = [f[id] for id in id_list_pos]
-            self.set_feature(fid, sf)
+        # popping out the former features and use the former id indices)
+        for feature_name in list(self.features):
+            current_feature = self.remove_feature(feature_name)
+            sf = [current_feature[id] for id in id_list_pos]
+            self.set_feature(feature_name, sf)
         # set new ROI features
         # (it's ok to do that after labels and id modification since we are
-        # poping out the former features and use the former id indices)
-        for fid in list(self.roi_features):
-            if fid != 'id':
-                f = self.remove_roi_feature(fid)
-                sf = np.ravel(f[int(id_list_pos)])
-                self.set_roi_feature(fid, sf)
+        # popping out the former features and use the former id indices)
+        for feature_name in list(self.roi_features):
+            if feature_name != 'id':
+                current_feature = self.remove_roi_feature(feature_name)
+                sf = np.ravel(current_feature[int(id_list_pos)])
+                self.set_roi_feature(feature_name, sf)
 
 
 def subdomain_from_array(labels, affine=None, nn=0):

nipy/labs/spatial_models/tests/test_mroi.py

@@ -79,16 +79,21 @@ def test_copy_subdomain():
 
 
 def test_select_roi():
-    """
-    """
+    # Test select_roi method
     mroi = make_subdomain()
     aux = np.random.randn(np.prod(shape))
     data = [aux[mroi.label == k] for k in range(8)]
     mroi.set_feature('data', data)
     mroi.set_roi_feature('data_mean', list(range(8)))
     mroi.select_roi([0])
     assert(mroi.k == 1)
+    assert_equal(mroi.roi_features['id'], [0])
     assert_equal(mroi.get_roi_feature('data_mean', 0), 0)
+    mroi.select_roi([])
+    assert(mroi.k == 0)
+    assert_equal(list(mroi.roi_features), ['id'])
+    assert_equal(mroi.roi_features['id'], [])
+
 
 def test_roi_features():
     """