BUG/API: Index.append with mixed object/Categorical indices

[jorisvandenbossche]

Closes #14298, related to #13660 and #13767

This closes the info() issue with CategoricalIndex, but the underlying issue is actually related to Index.append, when appending mixed dtype index objects:

In [11]: pd.Index(['a']).append(pd.CategoricalIndex(['a', 'b']))
...
AttributeError: 'Index' object has no attribute '_is_dtype_compat'

This error occurs when the calling index is not a Categorical (because of https://github.com/pandas-dev/pandas/blob/v0.19.0/pandas/indexes/base.py#L1439 the Categorical _is_dtype_compat method is used anyway).

But the question is, what should the result be of the above expression?

• a CategoricalIndex ? -> IMO not, as the calling index is not a CategoricalIndex, the result should also not be one (regardless of what is appended) -> so the intention of the code is IMO also not correct
• an object Index ? -> yes, this is also what 0.18.1 does

But, the more important discussion is what to do when the calling index is a CategoricalIndex ? In 0.18 (and in 0.19.0) this either returned a CategoricalIndex or raised an error:

In [13]: pd.__version__
Out[13]: u'0.18.1'

In [104]: pd.CategoricalIndex(['a', 'b']).append(pd.Index(['a']))
Out[104]: CategoricalIndex([u'a', u'b', u'a'], categories=[u'a', u'b'], ordered=False, dtype='category')

In [105]: pd.CategoricalIndex(['a', 'b']).append(pd.Index(['c']))
...
TypeError: cannot append a non-category item to a CategoricalIndex

But, for the concat case we decided to return an object dtyped Index in those cases instead of coercing or raising (see discussion in #13767 and the summary table with rules in #13767 (comment)). So the question is, do think that append should follow the same rules as concat. Or, do we want to be more flexible for append, and let it depend on the type of the calling Index?

cc @sinhrks

[jorisvandenbossche]

In the current PR, I just removed the check for category, which is the more intrusive change, as now the return result will always be object dtype if not self and all objects to append are CategoricalIndex.
I could also change the if 'category' in typs: check to if self.is_categorical():, which then keeps the current behaviour if the calling object is a CategoricalIndex.

[jreback]

then you should change this comment as this is no longer true (the Index must be a CI to return a CI) only.

[codecov-io]

Current coverage is 85.27% (diff: 100%)

Merging #14545 into master will not change coverage

@@             master     #14545   diff @@
==========================================
  Files           140        140          
  Lines         50693      50693          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          43229      43229          
  Misses         7464       7464          
  Partials          0          0          

Powered by Codecov. Last update 093aa82...0d47495

[jorisvandenbossche]

OK, so I changed the PR to only fix the issue when the calling Index is not of Categorical dtype (which caused the bug), and left the behaviour of CategoricalIndex.append(..) alone.
As a result pd.Index(['a']).append(pd.CategoricalIndex(['a', 'b'])) works again (returning object Index as in 0.18.1).

That is probably the easiest for now, to have this fix in 0.19.1, but we should then discuss the behaviour of Index.append(..) a bit more in detail for 0.20 (whether to follow the same rules as concat or not).

[TomAugspurger]

+1 on the behavior here (coercing Categorical to Index).

I'd feel slightly better if the monkey patching of sys.stdout was in a context manager

from contextlib import contextmanager

@contextmanager
def stdout():
    sys.stdout = StringIO()
    yield
    sys.stdout = sys.__stdout__

but not a huge deal.

[jorisvandenbossche]

I'd feel slightly better if the monkey patching of sys.stdout was in a context manager

I just copied the approach from other tests :-), so would be a nice clean-up in general!

[jreback]

don't re-invent the wheel, the pattern is:

buf = StringIO()
df.info(buf=buf, null_counts=null_counts)
self.assertTrue(('non-null' in buf.getvalue()) is result)

[jreback]

here

[jorisvandenbossche]

Updated the test