Repository: rsagroup/rsatoolbox

Issue:
`RDMs.sort_by()` take a `reindex` boolean argument to replace the `"index"` pattern descriptor
I just became rather confused because `RDMs.sort_by()` followed by `RDMs.subset_patterns(by="index", ...)` didn't have the expected effect.  In other words, sorting the conditions _also_ sorts `.pattern_descriptors["index"]`, so then `.subset_patterns(by="index", ...)` doesn't subset the patterns by their _current_ indices, but by their indices _prior to sorting_.

I'm sure there are many cases where it makes sense to sort the `"index"` pattern descriptor too, but in my case it'd be useful to automatically keep the patterns with an incremental index.  I propose `RDMs.sort_by()` takes a new boolean argument `reindex` (default `False`), which when `True` resets the `"index"` pattern descriptor to 0, 1, ..., n_cond-1.

Is there a reason or current usage of the `"index"` pattern descriptor which would make this problematic?

Discussion:

Patch:
diff --git a/src/rsatoolbox/rdm/rdms.py b/src/rsatoolbox/rdm/rdms.py
index 655ad3d7..6c3c7b4f 100644
--- a/src/rsatoolbox/rdm/rdms.py
+++ b/src/rsatoolbox/rdm/rdms.py
@@ -422,9 +422,13 @@ class RDMs:
         for dname, descriptors in self.pattern_descriptors.items():
             self.pattern_descriptors[dname] = [descriptors[idx] for idx in new_order]
 
-    def sort_by(self, **kwargs):
+    def sort_by(self, reindex: bool = True, **kwargs):
         """Reorder the patterns by sorting a descriptor
 
+        Args:
+            reindex (bool): whether to reset the 'index' descriptor 
+                following sorting
+
         Pass keyword arguments that correspond to descriptors,
         with value indicating the sort type. Supported methods:
 
@@ -466,6 +470,8 @@ class RDMs:
                 self.reorder([list(descriptor).index(x) for x in new_order])
             else:
                 raise ValueError(f'Unknown sorting method: {method}')
+        if reindex:
+            self.pattern_descriptors['index'] = list(range(self.n_cond))
 
     def mean(self, weights=None):
         """Average rdm of all rdms contained


Test patch:
diff --git a/tests/test_rdm.py b/tests/test_rdm.py
index b1429970..5a1c7287 100644
--- a/tests/test_rdm.py
+++ b/tests/test_rdm.py
@@ -429,6 +429,44 @@ class TestRDM(unittest.TestCase):
             )
         )
 
+    def test_sort_by_reindex_resets_index(self):
+        from rsatoolbox.rdm import RDMs
+        rdm = np.array([
+            [0., 1., 2., 3.],
+            [1., 0., 1., 2.],
+            [2., 1., 0., 1.],
+            [3., 2., 1., 0.]]
+        )
+        conds = ['b', 'a', 'c', 'd']
+        rdms = RDMs(
+            np.atleast_2d(squareform(rdm)),
+            pattern_descriptors=dict(conds=conds)
+        )
+        rdms.sort_by(conds='alpha', reindex=True)
+        self.assertEqual(
+            rdms.pattern_descriptors['index'],
+            list(range(rdms.n_cond))
+        )
+
+    def test_sort_by_not_reindex_does_not_reset_index(self):
+        from rsatoolbox.rdm import RDMs
+        rdm = np.array([
+            [0., 1., 2., 3.],
+            [1., 0., 1., 2.],
+            [2., 1., 0., 1.],
+            [3., 2., 1., 0.]]
+        )
+        conds = ['b', 'a', 'c', 'd']
+        rdms = RDMs(
+            np.atleast_2d(squareform(rdm)),
+            pattern_descriptors=dict(conds=conds)
+        )
+        rdms.sort_by(conds='alpha', reindex=False)
+        self.assertNotEqual(
+            rdms.pattern_descriptors['index'],
+            list(range(rdms.n_cond))
+        )
+
     def test_sort_by_list(self):
         from rsatoolbox.rdm import RDMs
         rdm = np.array([