Repository: kutaslab/fitgrid

Issue:
duplicate channel names cause trouble for slicing fit objects
after loading multichannel fitgrid as article_fg ...

works fine for channel slicer  ['MiPf', 'MiCe', 'MiPa', 'MiOc']

bombs if 'MiPa' is in the channel slicer twice

```
>>> infl = article_fg[-500:600,['MiPa', 'MiCe', 'MiPa', 'MiOc']].get_influence()
>>> cooks_d = infl.cooks_distance
```

```
AttributeError                            Traceback (most recent call last)
<ipython-input-156-91a10ace1480> in <module>()
      1 infl = article_fg[-500:600,['MiPa','MiCe', 'MiPa', 'MiOc']].get_influence()
----> 2 cooks_d = infl.cooks_distance
      3 
      4 #epoch_idxs = cooks_d.index.get_level_values('Epoch_idx').unique()
      5 #epoch_idxs

~/.conda/envs/UDCK18/lib/python3.6/site-packages/fitgrid/fitgrid.py in __getattr__(self, name)
    191 
    192         temp = self._grid.applymap(lambda x: getattr(x, name))
--> 193         return _expand(temp, self._epoch_index)
    194 
    195     def __call__(self, *args, **kwargs):

~/.conda/envs/UDCK18/lib/python3.6/site-packages/fitgrid/fitgrid.py in _expand(temp, epoch_index)
     86                 f'element has ndim = {array_form.ndim}.'
     87             )
---> 88         temp_expanded = expand_series_or_df(temp)
     89         temp_epoch_index = add_epoch_index(temp_expanded, epoch_index)
     90         return temp_epoch_index

~/.conda/envs/UDCK18/lib/python3.6/site-packages/fitgrid/fitgrid.py in expand_series_or_df(temp)
     45     )
     46     # concatenate columns, channel names are top level columns indices
---> 47     result = pd.concat(columns, axis=1, keys=temp.columns)
     48 
     49     # stack to achieve long form if columns have multiple levels

~/.conda/envs/UDCK18/lib/python3.6/site-packages/pandas/core/reshape/concat.py in concat(objs, axis, join, join_axes, ignore_index, keys, levels, names, verify_integrity, copy)
    210                        keys=keys, levels=levels, names=names,
    211                        verify_integrity=verify_integrity,
--> 212                        copy=copy)
    213     return op.get_result()
    214 

~/.conda/envs/UDCK18/lib/python3.6/site-packages/pandas/core/reshape/concat.py in __init__(self, objs, axis, join, join_axes, keys, levels, names, ignore_index, verify_integrity, copy)
    240             objs = [objs[k] for k in keys]
    241         else:
--> 242             objs = list(objs)
    243 
    244         if len(objs) == 0:

~/.conda/envs/UDCK18/lib/python3.6/site-packages/fitgrid/fitgrid.py in <genexpr>(.0)
     42 
     43     columns = (
---> 44         pd.concat(temp[channel].tolist(), keys=temp.index) for channel in temp
     45     )
     46     # concatenate columns, channel names are top level columns indices

~/.conda/envs/UDCK18/lib/python3.6/site-packages/pandas/core/generic.py in __getattr__(self, name)
   3612             if name in self._info_axis:
   3613                 return self[name]
-> 3614             return object.__getattribute__(self, name)
   3615 
   3616     def __setattr__(self, name, value):

AttributeError: 'DataFrame' object has no attribute 'tolist'
```

Discussion:

Patch:
diff --git a/fitgrid/epochs.py b/fitgrid/epochs.py
index 0485bf9..149821f 100644
--- a/fitgrid/epochs.py
+++ b/fitgrid/epochs.py
@@ -61,6 +61,12 @@ class Epochs:
                     f'{item} must be a column in the epochs table index.'
                 )
 
+        # check no duplicate column names in index and regular columns
+        names = list(epochs_table.index.names) + list(epochs_table.columns)
+        deduped_names = tools.deduplicate_list(names)
+        if deduped_names != names:
+            raise FitGridError('Duplicate column names not allowed.')
+
         # make our own copy so we are immune to modification to original table
         table = epochs_table.copy().reset_index().set_index(EPOCH_ID)
         assert table.index.names == [EPOCH_ID]
diff --git a/fitgrid/fitgrid.py b/fitgrid/fitgrid.py
index 4d50b48..b67fc2f 100644
--- a/fitgrid/fitgrid.py
+++ b/fitgrid/fitgrid.py
@@ -5,6 +5,7 @@ from collections import OrderedDict
 import warnings
 
 from .errors import FitGridError
+from . import tools
 
 
 def add_epoch_index(temp, epoch_index):
@@ -154,6 +155,11 @@ class FitGrid:
 
     def __init__(self, _grid, epoch_index):
 
+        # check no duplicate column names
+        names = list(_grid.columns)
+        deduped_names = tools.deduplicate_list(names)
+        if deduped_names != names:
+            raise FitGridError('Duplicate column names not allowed.')
         self._grid = _grid
         self._epoch_index = epoch_index
         self.tester = _grid.iloc[0, 0]
@@ -191,7 +197,7 @@ class FitGrid:
                 return component
             elif isinstance(component, list):
                 # deduplicate and warn if duplicates found
-                deduped_component = list(OrderedDict.fromkeys(component))
+                deduped_component = tools.deduplicate_list(component)
                 if deduped_component != component:
                     msg = f'Slicer {component} contained duplicates, '
                     msg += f'slicing instead on deduped: {deduped_component}.'
diff --git a/fitgrid/tools.py b/fitgrid/tools.py
index 5cd4e58..d0a2717 100644
--- a/fitgrid/tools.py
+++ b/fitgrid/tools.py
@@ -1,5 +1,5 @@
 import numpy as np
-from collections import defaultdict
+from collections import defaultdict, OrderedDict
 import subprocess
 import re
 import ctypes
@@ -7,7 +7,6 @@ import sys
 import os
 import glob
 import warnings
-from contextlib import contextmanager
 
 MKL = 'mkl'
 OPENBLAS = 'openblas'
@@ -45,6 +44,10 @@ def get_first_group(groupby):
     return first_group
 
 
+def deduplicate_list(lst):
+    return list(OrderedDict.fromkeys(lst))
+
+
 class BLAS:
     def __init__(self, cdll, kind):
 


Test patch:
diff --git a/tests/test_epochs.py b/tests/test_epochs.py
index 8d53aa6..c6caf74 100644
--- a/tests/test_epochs.py
+++ b/tests/test_epochs.py
@@ -1,7 +1,9 @@
 import pytest
 import numpy as np
+import pandas as pd
 from .context import fitgrid
-from fitgrid import fake_data, errors
+from fitgrid import fake_data
+from fitgrid.errors import FitGridError
 from fitgrid.epochs import Epochs
 from statsmodels.formula.api import ols
 import shutil
@@ -14,7 +16,7 @@ matplotlib.use('Agg')
 def test_epochs_lm_bad_inputs():
 
     epochs = fake_data.generate()
-    with pytest.raises(errors.FitGridError):
+    with pytest.raises(FitGridError):
         epochs.lm(LHS=['bad_channel'], RHS='categorical')
 
 
@@ -25,11 +27,24 @@ def test_epochs_unequal_snapshots():
     )
 
     epochs_table.drop(epochs_table.index[42], inplace=True)
-    with pytest.raises(errors.FitGridError) as error:
+    with pytest.raises(FitGridError) as error:
         Epochs(epochs_table, channels)
     assert 'differs from previous snapshot' in str(error.value)
 
 
+def test_raises_error_on_duplicate_channels():
+
+    epochs_table, channels = fitgrid.fake_data._generate(10, 100, 2, 32)
+    dupe_channel = channels[0]
+    dupe_column = epochs_table[dupe_channel]
+    bad_epochs_table = pd.concat([epochs_table, dupe_column], axis=1)
+
+    with pytest.raises(FitGridError) as error:
+        fitgrid.epochs_from_dataframe(bad_epochs_table, channels)
+
+    assert "Duplicate column names" in str(error.value)
+
+
 def test__raises_error_on_epoch_index_mismatch():
     """Bad: all epochs have the same shape, but indices differ."""
 
@@ -46,7 +61,7 @@ def test__raises_error_on_epoch_index_mismatch():
     epochs_table.index.set_labels(labels=bad_index, level=TIME, inplace=True)
 
     # now time index is equal to row number in the table overall
-    with pytest.raises(errors.FitGridError) as error:
+    with pytest.raises(FitGridError) as error:
         Epochs(epochs_table, channels)
 
     assert 'differs from previous snapshot' in str(error.value)
diff --git a/tests/test_fitgrid.py b/tests/test_fitgrid.py
index f256a86..680e53f 100644
--- a/tests/test_fitgrid.py
+++ b/tests/test_fitgrid.py
@@ -1,7 +1,9 @@
 import pytest
 import numpy as np
+import pandas as pd
 from .context import fitgrid
 from fitgrid.errors import FitGridError
+from fitgrid.fitgrid import FitGrid
 from fitgrid import tools
 import matplotlib
 
@@ -151,6 +153,22 @@ def test__slicing():
         grid[['channel0', 'channel1']]
 
 
+def test_grid_with_duplicate_channels():
+
+    epochs = fitgrid.generate()
+    grid = epochs.lm(
+        LHS=['channel0', 'channel1', 'channel2'],
+        RHS='categorical + continuous',
+    )
+    _grid_with_duplicate_channels = pd.concat(
+        [grid._grid, grid._grid['channel0']], axis=1
+    )
+    with pytest.raises(FitGridError) as error:
+        FitGrid(_grid_with_duplicate_channels, grid._epoch_index)
+
+    assert "Duplicate column names" in str(error.value)
+
+
 def test__smoke_influential_epochs():
 
     epochs = fitgrid.generate()
diff --git a/tests/test_tools.py b/tests/test_tools.py
index f341650..851ba69 100644
--- a/tests/test_tools.py
+++ b/tests/test_tools.py
@@ -19,6 +19,13 @@ def test_duplicate_index_printer():
     assert re.search(r'2\s+2, 4\n', table) is not None
 
 
+def test_deduplicate_list():
+
+    # should remove duplicates and preserve order
+    l = [1, 1, 2, 3, 1, 2, 4]
+    assert tools.deduplicate_list(l) == [1, 2, 3, 4]
+
+
 def test_blas_getter():
 
     import numpy