Multi-dimensional Index indexing followups

[TomAugspurger]

• Avoid DeprecationWarning followed by exception: BUG: validate Index data is 1D + deprecate multi-dim indexing #30588 (comment)
• Update tests to filter warnings in plotting https://github.com/pandas-dev/pandas/pull/30588/files/74dffbedacb34bde7e5a8fb2396e0fef9efac499#diff-4b6d08e53050950f83ea75e432302f89
• Change CategoricalIndex behavior to raise, matching IntervalIndex BUG: validate Index data is 1D + deprecate multi-dim indexing #30588 (comment)
• Deprecate it in Series.__getitem__ as well, similar to Index.__getitem__

[jorisvandenbossche]

I added deprecation for the Series case

[jorisvandenbossche]

Currently, you get the following warning:

In [3]: warnings.simplefilter("always")  

In [5]: s = pd.Series([1, 2, 3], index=[1, 2, 3]) 

In [6]: s[:, None] 
/home/joris/scipy/pandas/pandas/core/internals/managers.py:1513: DeprecationWarning: Support for multi-dimensional indexing (e.g. `index[:, None]`) on an Index is deprecated and will be removed in a future version.  Convert to a numpy array before indexing instead.
  
Out[6]: 
array([[1],
       [2],
       [3]])

which is not very friendly. Eg when running tests you get:

  /home/travis/build/statsmodels/statsmodels/venv/lib/python3.7/site-packages/pandas/core/internals/managers.py:1542: DeprecationWarning: Support for multi-dimensional indexing (e.g. `index[:, None]`) on an Index is deprecated and will be removed in a future version.  Convert to a numpy array before indexing instead.

    return type(self)(self._block._slice(slobj), self.index[slobj], fastpath=True)

which seems like that the deprecation warning is coming from code in pandas itself (which also is the case, it's just that indexing Series with 2d then also indexes into the Index). You get a message about the index, while you are using a Series.
(those warning messages come up in seaborn / statsmodels test suite)

So we should add a proper message for Series itself.
Not sure if there would be a way to avoid the double warning and when doing s[:, None] to only get a warning about the Series, and not about the Index

[jorisvandenbossche]

I think this is a blocker as well cc @TomAugspurger @jbrockmendel

Or at least the non-test-clean-up part (so eg Series deprecation)

[tacaswell]

Can the exception raised be IndexError or TypeError as per https://github.com/matplotlib/matplotlib/pull/15007/files#r312455732 ? From quickly skimming #30588 it looks like it is ValueError, sorry for the noise if I am confused.

[jorisvandenbossche]

For now, it should only raise a deprecation warning, and not yet an error (the error in the linked PR is if you explicitly construct an Index with eg Index(..) and passing it 2D data; the 2D indexing (idx[:, None]) is only deprecated)

[tacaswell]

It is clear that it is just warning for now, but on the Matplotlib side I am banking on it raising either IndexError or TypeError (as it will "just work" for us in that case). If you want to raise something different, would it be possible to decide that now so we don't have a crisis when it does change over to raising ;)

[jorisvandenbossche]

Ah, OK, I understand.
I would say either IndexError or ValueError, but I think IndexError can do (it's what you get in numpy if you index with too many dimensions, although here it's a bit different of course, as expanding dimensions is normally allowed in numy)

@jbrockmendel opinion on this?

[jbrockmendel]

opinion on this?

IndexError strikes me as weird here, but if numpy does it I guess we can go along. ValueError sounds a bit better than TypeError, but I agree that choosing something and being consistent is more important than the exact choice.

Another alternative would be to raise InvalidIndexError (which we can document the meaning of more precisely), and have that subclass LookupError for matplotlib to catch.

[tacaswell]

For xref purposes matplotlib/matplotlib#16347 is how we are currently going to handle this.

[TomAugspurger]

All the 1.0 related items have been fixed I think.

[jeandersonbc]

I noticed that I did a mistake when I was synchronizing my fork and I was added as one of the authors in the later commit reference (as seen above). Please, ignore it.

[mroeschke]

I think all the issues in the top post have been addressed so closing. Can reopen individual issues if any are outstanding