Repository: glue-viz/glue

Issue:
Allow categorical components to be >1D
**Is your feature request related to a problem? Please describe it:**
Categorical components are currently limited to be 1D, but it is sometimes useful to have categorical components with higher dimension. 

**Describe the solution you'd like:**
Enable categorical components with >1D, perhaps by casting the input array to 1D, using the current pandas functions to transform and then casting back into the original shape.

**Additional context:**
Originally posted as a request in Slack.


Discussion:
jfoster17: What is your preferred way for me to make additions to this? A PR into astrofrog:categorical-nd, or just pasting here? 

You need to update `core.component.autotyped`

`n = coerce_numeric(data.ravel()).reshape(data.shape)`

Proposed addition to `test_component.py`

```
def test_nd_categorical_component_autotype():
    x = np.array([['M', 'F'], ['F', 'M']])
    assert Component.autotyped(x).categorical

    d = Data(x=[['male', 'female', 'male'],['female','female','male']])
    assert d.get_component('x').categorical
```


jfoster17: This still doesn't do exactly what I would want to it do when you combine a 2D image of some matrix data with a 1D histogram of the categorical components. Specifically, making a subset on the 2D image does make a selection on the categorical array that properly updates the histogram but trying to make a subset on the 1D histogram to (for example) select all the points in the 2D image with a given categorical value does not work:


```Traceback (most recent call last):
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/backends/backend_qt5.py", line 480, in _draw_idle
    self.draw()
  File "/Users/jfoster/glue/glue/viewers/matplotlib/qt/widget.py", line 124, in draw
    return super(MplCanvas, self).draw(*args, **kwargs)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/backends/backend_agg.py", line 407, in draw
    self.figure.draw(self.renderer)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/artist.py", line 41, in draw_wrapper
    return draw(artist, renderer, *args, **kwargs)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/figure.py", line 1863, in draw
    mimage._draw_list_compositing_images(
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/image.py", line 131, in _draw_list_compositing_images
    a.draw(renderer)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/astropy/visualization/wcsaxes/core.py", line 459, in draw
    super().draw(renderer, **kwargs)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/artist.py", line 41, in draw_wrapper
    return draw(artist, renderer, *args, **kwargs)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/cbook/deprecation.py", line 411, in wrapper
    return func(*inner_args, **inner_kwargs)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/axes/_base.py", line 2747, in draw
    mimage._draw_list_compositing_images(renderer, self, artists)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/image.py", line 131, in _draw_list_compositing_images
    a.draw(renderer)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/artist.py", line 41, in draw_wrapper
    return draw(artist, renderer, *args, **kwargs)
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/matplotlib/image.py", line 643, in draw
    im, l, b, trans = self.make_image(
  File "/Users/jfoster/opt/anaconda3/envs/glueviz-dev/lib/python3.9/site-packages/mpl_scatter_density/base_image_artist.py", line 177, in make_image
    array = self._array_func(bins=bins, range=((ymin, ymax), (xmin, xmax)))
  File "/Users/jfoster/glue/glue/viewers/image/frb_artist.py", line 27, in array_func_wrapper
    array = self._array_maker(bounds)
  File "/Users/jfoster/glue/glue/viewers/image/layer_artist.py", line 245, in __call__
    mask = self.layer_state.get_sliced_data(bounds=bounds)
  File "/Users/jfoster/glue/glue/viewers/image/state.py", line 455, in get_sliced_data
    image = self.layer.data.compute_fixed_resolution_buffer(full_view, target_data=self.viewer_state.reference_data,
  File "/Users/jfoster/glue/glue/core/data.py", line 1945, in compute_fixed_resolution_buffer
    return compute_fixed_resolution_buffer(self, *args, **kwargs)
  File "/Users/jfoster/glue/glue/core/fixed_resolution_buffer.py", line 274, in compute_fixed_resolution_buffer
    array[invalid_all] = invalid_value
IndexError: too many indices for array: array is 1-dimensional, but 2 were indexed
```

The interaction I describe above does work if the 2D component is, e.g. a 2D array of 1/0 instead of a full CategoricalComponent.
jfoster17: The above error message is a bit leading, it is actually coming from the 2D image viewer I had open. 

The core problem actually seems to be that selecting a portion of a categorical 1D histogram where the underlying data is >1D  leads to a 1D subset (at least with to_mask()).


**NB: this will only work with my fix to autotyped posted above.**
```
from glue.core import Data
data = Data(x=[['Male','Female','Male'],['Female','Male','Male']],label='test')
dc.append(data)
```
I then create a 1D histogram and try to select the Females. This creates a subset without issuing an error, but the subset does not display. This is probably related to the fact that:

```
In [11]: dc[0].subsets[0].to_mask()
Out[11]: array([False,  True, False,  True, False, False])
```
Whereas doing it at the command line does the correct thing:

```
state = data.id['x'] == 'Male'
subset_group = dc.new_subset_group('Male',state)
dc[0].subsets[1].to_mask()

```
```
array([[ True, False,  True],
       [False,  True,  True]])
```
jfoster17: I think I have fixed the masking problem with Nd categoricals. The branch/fork here:

https://github.com/gluesolutions/glue/tree/categorical-nd

Shows that solution (just remove a .ravel() statement) and includes a test that failed before and now works. 
astrofrog: @jfoster17 - thanks! I've rebased this and included your change here so let's see if everything passes now.
astrofrog: The CI issues are real, somehow some of the changes here are causing a massive slowdown somewhere which is causing the CI to time out.
astrofrog: I think I figured it out, let's see what the CI says
codecov[bot]: # [Codecov](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) Report
> Merging [#2214](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) (2920829) into [master](https://codecov.io/gh/glue-viz/glue/commit/3719c4ffeccdabe974f201983ee35c0a01789ffa?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) (3719c4f) will **decrease** coverage by `0.92%`.
> The diff coverage is `78.94%`.

[![Impacted file tree graph](https://codecov.io/gh/glue-viz/glue/pull/2214/graphs/tree.svg?width=650&height=150&src=pr&token=zYo33FJ9rS&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz)](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz)

```diff
@@            Coverage Diff             @@
##           master    #2214      +/-   ##
==========================================
- Coverage   88.07%   87.14%   -0.93%     
==========================================
  Files         247      247              
  Lines       23039    22427     -612     
==========================================
- Hits        20291    19544     -747     
- Misses       2748     2883     +135     
```


| [Impacted Files](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) | Coverage Δ | |
|---|---|---|
| [glue/core/data\_collection.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS9jb3JlL2RhdGFfY29sbGVjdGlvbi5weQ==) | `87.93% <69.23%> (-4.57%)` | :arrow_down: |
| [glue/core/component.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS9jb3JlL2NvbXBvbmVudC5weQ==) | `88.94% <100.00%> (-7.33%)` | :arrow_down: |
| [glue/core/subset.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS9jb3JlL3N1YnNldC5weQ==) | `88.23% <100.00%> (-1.60%)` | :arrow_down: |
| [glue/utils/array.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS91dGlscy9hcnJheS5weQ==) | `89.37% <100.00%> (-0.12%)` | :arrow_down: |
| [glue/plugins/dendro\_viewer/data\_factory.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS9wbHVnaW5zL2RlbmRyb192aWV3ZXIvZGF0YV9mYWN0b3J5LnB5) | `4.65% <0.00%> (-95.35%)` | :arrow_down: |
| [glue/core/data\_factories/image.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS9jb3JlL2RhdGFfZmFjdG9yaWVzL2ltYWdlLnB5) | `22.50% <0.00%> (-72.63%)` | :arrow_down: |
| [glue/conftest.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS9jb25mdGVzdC5weQ==) | `53.03% <0.00%> (-9.10%)` | :arrow_down: |
| [glue/utils/qt/app.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS91dGlscy9xdC9hcHAucHk=) | `88.00% <0.00%> (-8.00%)` | :arrow_down: |
| [glue/utils/matplotlib.py](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz#diff-Z2x1ZS91dGlscy9tYXRwbG90bGliLnB5) | `83.24% <0.00%> (-7.04%)` | :arrow_down: |
| ... and [98 more](https://codecov.io/gh/glue-viz/glue/pull/2214/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz). Last update [3719c4f...2920829](https://codecov.io/gh/glue-viz/glue/pull/2214?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glue-viz).


Patch:
diff --git a/CHANGES.md b/CHANGES.md
index 8f4073e9..14b1ab4c 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -12,6 +12,8 @@ v1.2.0 (unreleased)
 * Expose ``DataCollection.delay_link_manager_update`` which can be used
   to delay any updating to the link tree when adding datasets. [#2225]
 
+* Allow CategoricalComponents to be n-dimensional. [#2214]
+
 v1.1.0 (2021-07-21)
 -------------------
 
diff --git a/glue/core/component.py b/glue/core/component.py
index bb958a02..57333db8 100644
--- a/glue/core/component.py
+++ b/glue/core/component.py
@@ -148,7 +148,7 @@ class Component(object):
         if data.dtype.kind == 'M':
             return DateTimeComponent(data)
 
-        n = coerce_numeric(data)
+        n = coerce_numeric(data.ravel()).reshape(data.shape)
 
         thresh = 0.5
         try:
@@ -389,8 +389,8 @@ class CategoricalComponent(Component):
 
         self._data = categorical_ndarray(categorical_data, copy=False, categories=categories)
 
-        if self._data.ndim != 1:
-            raise ValueError("Categorical Data must be 1-dimensional")
+        if self._data.ndim < 1:
+            raise ValueError("Categorical Data must be at least 1-dimensional")
 
         self.jitter(method=jitter)
 
@@ -435,7 +435,7 @@ class CategoricalComponent(Component):
         Parameters
         ----------
         method : {None, 'uniform'}
-            If `None`, not jittering is done (or any jittering is undone).
+            If `None`, no jittering is done (or any jittering is undone).
             If ``'uniform'``, the codes are randomized by a uniformly
             distributed random variable.
         """
diff --git a/glue/core/subset.py b/glue/core/subset.py
index 28d8398a..0021c22d 100644
--- a/glue/core/subset.py
+++ b/glue/core/subset.py
@@ -684,7 +684,7 @@ class CategoricalROISubsetState(SubsetState):
         x = data[self.att, view]
         result = self.roi.contains(x, None)
         assert x.shape == result.shape
-        return result.ravel()
+        return result
 
     def copy(self):
         result = CategoricalROISubsetState()
diff --git a/glue/utils/array.py b/glue/utils/array.py
index 82afcaa4..1cb4b377 100644
--- a/glue/utils/array.py
+++ b/glue/utils/array.py
@@ -57,8 +57,8 @@ def unique(array):
     # numpy.unique doesn't handle mixed-types on python3,
     # so we use pandas
     array = np.asarray(array)
-    I, U = pd.factorize(array, sort=True)
-    return U.astype(array.dtype), I
+    I, U = pd.factorize(array.ravel(), sort=True)
+    return U.astype(array.dtype), I.reshape(array.shape)
 
 
 def shape_to_string(shape):


Test patch:
diff --git a/glue/core/tests/test_component.py b/glue/core/tests/test_component.py
index 6f41fb3d..550d3148 100644
--- a/glue/core/tests/test_component.py
+++ b/glue/core/tests/test_component.py
@@ -183,6 +183,23 @@ class TestCategoricalComponent(object):
             cat_comp.jitter(method='this will never be a jitter method')
 
 
+def test_nd_categorical_component():
+    data = [['a', 'b'], ['c', 'b']]
+    cat_comp = CategoricalComponent(data)
+    np.testing.assert_equal(cat_comp.data, data)
+    np.testing.assert_equal(cat_comp.labels, data)
+    np.testing.assert_equal(cat_comp.codes, [[0, 1], [2, 1]])
+    np.testing.assert_equal(cat_comp.categories, ['a', 'b', 'c'])
+
+
+def test_nd_categorical_component_autotype():
+    x = np.array([['M', 'F'], ['F', 'M']])
+    assert Component.autotyped(x).categorical
+
+    d = Data(x=[['male', 'female', 'male'], ['female', 'female', 'male']])
+    assert d.get_component('x').categorical
+
+
 class TestCoordinateComponent(object):
 
     def setup_method(self, method):
diff --git a/glue/core/tests/test_subset.py b/glue/core/tests/test_subset.py
index b33cdad8..e953efc0 100644
--- a/glue/core/tests/test_subset.py
+++ b/glue/core/tests/test_subset.py
@@ -693,6 +693,18 @@ def test_inherited_properties():
     assert sub.hub is d.hub
 
 
+class TestNDCategoricalSubsets():
+
+    def setup_method(self, method):
+        self.data = Data(sex=[['Male', 'Male', 'Female'], ['Female', 'Male', 'Female']])
+
+    def test_categorical_roi_subset(self):
+        roi = CategoricalROI(['sex', 'Male'])
+        subset = self.data.new_subset()
+        subset.subset_state = CategoricalROISubsetState(att=self.data.id['sex'], roi=roi)
+        np.testing.assert_equal(self.data.subsets[0].to_mask(), [[1, 1, 0], [0, 1, 0]])
+
+
 class TestCloneSubsetStates():
 
     def setup_method(self, method):
diff --git a/glue/utils/tests/test_array.py b/glue/utils/tests/test_array.py
index b6765d75..adfef7a2 100644
--- a/glue/utils/tests/test_array.py
+++ b/glue/utils/tests/test_array.py
@@ -36,6 +36,14 @@ def test_unique_dtype():
     assert U.dtype.kind in 'SU'
 
 
+def test_unique_nd():
+
+    array = np.array([['a', 'b'], ['c', 'a']])
+    U, I = unique(array)
+    assert_equal(U, ['a', 'b', 'c'])
+    assert_equal(I, [[0, 1], [2, 0]])
+
+
 def test_shape_to_string():
     assert shape_to_string((1, 4, 3)) == "(1, 4, 3)"