Repository: rmarkello/abagen

Issue:
Allow single hemisphere mirroring with `lr_mirror`
## The issue

As of #87, `abagen.get_expression_data()` accepts the `lr_mirror` parameter (a boolean; default: False) that will, if set, mirror tissue samples bilaterally across hemispheres, effectively duplicating the number of samples and dramatically increasing the spatial coverage of the AHBA dataset. 

There are some instances, however, where it might be desirable to not mirror samples bilaterally and rather only mirror left --> right or right --> left.

## Proposed solution

Modify the `lr_mirror` parameter to accept 'left' and 'right', which would specify the SOURCE hemisphere from which samples would be mirrored. 

That's might be a bit confusing (though the documentation should be clear), so maybe it would be good to also accept some other inputs (e.g., 'lr' would mean 'mirror left to right' and 'rl' would be the opposite)? Open to suggestions on what exactly the choices would be such that this would make sense and be the least confusing. 

I still think by default any form of mirroring should be OFF, but it would be nice to expose these additional options!


Discussion:

Patch:
diff --git a/abagen/allen.py b/abagen/allen.py
index 52cb4bf..5fb7497 100644
--- a/abagen/allen.py
+++ b/abagen/allen.py
@@ -4,6 +4,7 @@ Functions for mapping AHBA microarray dataset to atlases and and parcellations
 """
 
 from functools import reduce
+import warnings
 
 import nibabel as nib
 import numpy as np
@@ -26,7 +27,7 @@ def get_expression_data(atlas,
                         ibf_threshold=0.5,
                         probe_selection='diff_stability',
                         donor_probes='aggregate',
-                        lr_mirror=False,
+                        lr_mirror=None,
                         exact=True,
                         tolerance=2,
                         sample_norm='srs',
@@ -117,11 +118,12 @@ def get_expression_data(atlas,
         donor ('independent'), or based on the most common selected probe
         across donors ('common'). Not all combinations of `probe_selection`
         and `donor_probes` methods are viable. Default: 'aggregate'
-    lr_mirror : bool, optional
+    lr_mirror : {None, 'bidirectional', 'leftright', 'rightleft'}, optional
         Whether to mirror microarray expression samples across hemispheres to
-        increase spatial coverage. This will duplicate samples across both
-        hemispheres (i.e., L->R and R->L), approximately doubling the number of
-        available samples. Default: False
+        increase spatial coverage. Using 'bidirectional' will mirror samples
+        across both hemispheres, 'leftright' will mirror samples in the left
+        hemisphere to the right, and 'rightleft' will mirror the right to the
+        left. Default: 'bidirectional'
     exact : bool, optional
         Whether to use exact matching of donor tissue samples to parcels in
         `atlas`. If True, this function will ONLY match tissue samples to
@@ -328,6 +330,19 @@ def get_expression_data(atlas,
                          f'one of [\'aggregate\', \'independent\']. Received '
                          f'value: \'{donor_probes}\'')
 
+    if isinstance(lr_mirror, bool):
+        warnings.warn('Setting lr_mirror to a boolean value will be '
+                      'deprecated in an upcoming release. Use either '
+                      '`lr_mirror=None` or `lr_mirror="bidirectional"` '
+                      'instead. See documentation for more details.',
+                      DeprecationWarning, stacklevel=2)
+        lr_mirror = 'bidirectional' if lr_mirror else None
+
+    mirror_opts = (None, 'bidirectional', 'leftright', 'rightleft')
+    if lr_mirror not in mirror_opts:
+        raise ValueError('Provided lr_mirror method is invalid, must be one '
+                         f'of {mirror_opts}. Received value: \'{lr_mirror}\'')
+
     # fetch files (downloading if necessary) and unpack to variables
     files = datasets.fetch_microarray(data_dir=data_dir, donors=donors,
                                       verbose=verbose, n_proc=n_proc)
@@ -360,8 +375,8 @@ def get_expression_data(atlas,
         annot = samples_.update_coords(annot, corrected_mni=corrected_mni,
                                        native_space=t1w)
         annot = samples_.drop_mismatch_samples(annot, ontol)
-        if lr_mirror:
-            annot = samples_.mirror_samples(annot, ontol)
+        if lr_mirror is not None:
+            annot = samples_.mirror_samples(annot, ontol, swap=lr_mirror)
         data['annotation'] = annot
     annotation = flatten_dict(files, 'annotation')
 
@@ -392,10 +407,10 @@ def get_expression_data(atlas,
                           index=np.append([0], all_labels),
                           columns=microarray.keys())
     for subj in microarray:
-        if lr_mirror:  # reset index (duplicates will cause issues if we don't)
+        if lr_mirror is not None:  # reset index
             # TODO: come up with alternative sample IDs for mirrored samples
-            annotation[subj] = annotation[subj].reset_index(drop=True)
             microarray[subj] = microarray[subj].reset_index(drop=True)
+            annotation[subj] = annotation[subj].reset_index(drop=True)
 
         # assign samples to regions
         labels = atlas[subj].label_samples(annotation[subj], tolerance)
diff --git a/abagen/samples_.py b/abagen/samples_.py
index b5791bd..827cf0e 100644
--- a/abagen/samples_.py
+++ b/abagen/samples_.py
@@ -267,7 +267,7 @@ def _mirror_ontology(annotation, ontology):
     return annotation
 
 
-def mirror_samples(annotation, ontology):
+def mirror_samples(annotation, ontology, swap='bidirectional'):
     """
     Mirrors all tissue samples across hemispheres (L->R and R->L)
 
@@ -284,6 +284,10 @@ def mirror_samples(annotation, ontology):
         Filepath to ontology file from Allen Brain Institute (i.e., as
         obtained by calling :func:`abagen.fetch_microarray` and accessing the
         `ontology` attribute on the resulting object).
+    swap : {'bidirectional', 'leftright', 'rightleft'}, optional
+        Which hemispheres to mirror, where 'bidirectional' will mirror both
+        hemispheres, 'leftright' will mirror the left to the right, and
+        'rightleft' will mirror the right to the left. Default: 'bidirectional'
 
     Returns
     -------
@@ -306,18 +310,24 @@ def mirror_samples(annotation, ontology):
        layers of the human cortex. NeuroImage, 171, 256-267.
     """
 
+    avail_swap = ('bidirectional', 'leftright', 'rightleft')
+    if swap not in avail_swap:
+        raise ValueError('Provided hemisphere `swap` value not recognized. '
+                         f'Must be one of {avail_swap}. Received: {swap}')
+
     annotation = io.read_annotation(annotation)
     ontology = io.read_ontology(ontology)
 
     # take all lh and rh samples and flip x-coordinate
     # also update ontology information (structure_id/acronym/name) as this is
     # used when dropping mismatched samples later in the workflow
-    lh = _mirror_ontology(annotation[annotation['mni_x'] < 0], ontology)
-    rh = _mirror_ontology(annotation[annotation['mni_x'] > 0], ontology)
-
-    # update mni-x coordinate (simply flip the sign)
-    for df in [lh, rh]:
-        df['mni_x'] *= -1
+    lh = rh = pd.DataFrame()
+    if swap in ('leftright', 'bidirectional'):
+        lh = _mirror_ontology(annotation[annotation['mni_x'] < 0], ontology)
+        lh['mni_x'] *= -1
+    if swap in ('rightleft', 'bidirectional'):
+        rh = _mirror_ontology(annotation[annotation['mni_x'] > 0], ontology)
+        rh['mni_x'] *= -1
 
     # grow microarray and pacall based on duplicated samples
     annotation = pd.concat([annotation, lh, rh])
diff --git a/docs/user_guide/expression.rst b/docs/user_guide/expression.rst
index eae04cc..1d2ac01 100644
--- a/docs/user_guide/expression.rst
+++ b/docs/user_guide/expression.rst
@@ -189,20 +189,26 @@ regions in the left hemisphere (for which all six donors have tissue samples)
 will be matched to at least one sample, whereas regions in the right hemisphere
 may come up short.
 
-To remedy this you can try setting the ``lr_mirror`` parameter to ``True`` when
-calling :func:`abagen.get_expression_data`. This, as the name suggests,
-performs a left/right mirroring of all the tissue samples from all donors. That
-is, all samples in the left hemisphere are duplicated and mirrored onto the
-right hemisphere, and vice-versa for right to left. Unlike the ``exact=False``
-parameter this will not *guarantee* that all regions are matched to a sample,
-but it will dramatically increase the likelihood that this will happen:
+To remedy this you can try modifying the ``lr_mirror`` parameter when calling
+:func:`abagen.get_expression_data`. This parameter accepts four options:
+``None`` (default), ``"bidirectional"``, ``"leftright"``, and ``"rightleft"``.
+As the name suggests, the ``lr_mirror`` options control whether tissue samples
+are mirrored across the left/right hemisphere axis. By supplying the
+'bidirectional' options, all samples in the left hemisphere are duplicated and
+mirrored onto the right hemisphre, and vice-versa for right to left. The other
+options ('leftright' and 'rightleft) will mirror only one hemisphere (i.e.,
+'leftright' will mirror samples in the left onto the right hemisphere.
+
+Unlike the ``exact=False`` parameter this will *not guarantee* that all regions
+are matched to a sample, but it will dramatically increase the likelihood that
+this will happen:
 
 .. insert figure demonstrating duplication of samples across hemispheres
 
 .. doctest::
     :options: +SKIP
 
-    >>> exp_mirror = abagen.get_expression_data(atlas['image'], atlas['info'], lr_mirror=True)
+    >>> exp_mirror = abagen.get_expression_data(atlas['image'], atlas['info'], lr_mirror='bidirectional')
     >>> print(exp_mirror)
     gene_symbol      A1BG  A1BG-AS1       A2M  ...       ZYX     ZZEF1      ZZZ3
     label                                      ...


Test patch:
diff --git a/abagen/tests/test_samples.py b/abagen/tests/test_samples.py
index 9622011..dbc1d0a 100644
--- a/abagen/tests/test_samples.py
+++ b/abagen/tests/test_samples.py
@@ -172,6 +172,23 @@ def test_mirror_samples(annotation, ontology):
     a = samples_.mirror_samples(annotation, ontology)
     pd.testing.assert_frame_equal(a, aexp, check_like=True)
 
+    b = samples_.mirror_samples(annotation, ontology, 'leftright')
+    pd.testing.assert_frame_equal(b, aexp.iloc[[0, 1, 2, 3]], check_like=True)
+
+    c = samples_.mirror_samples(annotation, ontology, 'rightleft')
+    pd.testing.assert_frame_equal(c, aexp.iloc[[0, 1, 2, 4]], check_like=True)
+
+    right_only = annotation.iloc[[1, 2]].copy()
+    d = samples_.mirror_samples(right_only, ontology, 'leftright')
+    pd.testing.assert_frame_equal(d, aexp.iloc[[1, 2]], check_like=True)
+
+    left_only = annotation.iloc[[0, 2]].copy()
+    e = samples_.mirror_samples(left_only, ontology, 'rightleft')
+    pd.testing.assert_frame_equal(e, aexp.iloc[[0, 2]], check_like=True)
+
+    with pytest.raises(ValueError):
+        samples_.mirror_samples(annotation, ontology, 'notaswap')
+
 
 def test_groupby_index():
     # default usage (no params)