• Avoid DeprecationWarning followed by exception: https://github.com/pandas-dev/pandas/pull/30588#discussion_r364930269
  • 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 https://github.com/pandas-dev/pandas/pull/30588#discussion_r364924210
  • Deprecate it in Series.__getitem__ as well, similar to Index.__getitem__

Comment From: jorisvandenbossche

I added deprecation for the Series case

Comment From: 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

Comment From: 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)

Comment From: 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.

Comment From: 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)

Comment From: 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 ;)

Comment From: 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?

Comment From: 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.

Comment From: tacaswell

For xref purposes https://github.com/matplotlib/matplotlib/pull/16347 is how we are currently going to handle this.

Comment From: TomAugspurger

All the 1.0 related items have been fixed I think.

Comment From: 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.

Comment From: mroeschke

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