Drop & insert on subtypes of index return their subtypes
State: closed

Current behavior:

``` python
In [32]:
period_index=pd.PeriodIndex(start='2015-01-01',end='2015-01-07',freq='B')
period_index
Out[32]:
PeriodIndex(['2015-01-01', '2015-01-02', '2015-01-05', '2015-01-06',
             '2015-01-07'],
            dtype='int64', freq='B')

In [33]:
period_index.drop(period_index[:2])
Out[33]:
Int64Index([11742, 11743, 11744], dtype='int64')
```

New behavior:

``` python
In [32]: period_index.drop(period_index[:2])
Out[32]: PeriodIndex(['2015-01-05', '2015-01-06', '2015-01-07'], dtype='int64', freq='B')
```

I could combine with #10599, but am having a couple of issues with this (will post below) that I need help with - I'll take direction on whether to combine or not.


Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Period
description: Period data type

Comments:
these can be independent 

The proposal is to replace using Index constructors with `._shallow_copy`, which maintains the type and retains the attributes on the index.

In general this works great. But there are a couple of corner cases I can't figure out. Specifically:

``` python

In [34]:
period_index=pd.PeriodIndex(start='2015-01-01',end='2015-01-30',freq='B')
In [35]:
two_periods_index=period_index._shallow_copy(period_index[5:7])
In [38]:
two_periods_index.equals(period_index[5:7])
Out[38]:
True
```

...which is as expected. But when I try and print `two_periods_index`, I get this error, which I haven't been able to attribute yet:

``` python
In [39]:
two_periods_index

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
/usr/local/lib/python2.7/dist-packages/IPython/core/formatters.pyc in __call__(self, obj)
    688                 type_pprinters=self.type_printers,
    689                 deferred_pprinters=self.deferred_printers)
--> 690             printer.pretty(obj)
    691             printer.flush()
    692             return stream.getvalue()

/usr/local/lib/python2.7/dist-packages/IPython/lib/pretty.pyc in pretty(self, obj)
    407                             if callable(meth):
    408                                 return meth(obj, self, cycle)
--> 409             return _default_pprint(obj, self, cycle)
    410         finally:
    411             self.end_group()

/usr/local/lib/python2.7/dist-packages/IPython/lib/pretty.pyc in _default_pprint(obj, p, cycle)
    527     if _safe_getattr(klass, '__repr__', None) not in _baseclass_reprs:
    528         # A user-provided repr. Find newlines and replace them with p.break_()
--> 529         _repr_pprint(obj, p, cycle)
    530         return
    531     p.begin_group(1, '<')

/usr/local/lib/python2.7/dist-packages/IPython/lib/pretty.pyc in _repr_pprint(obj, p, cycle)
    709     """A pprint that just redirects to the normal repr function."""
    710     # Find newlines and replace them with p.break_()
--> 711     output = repr(obj)
    712     for idx,output_line in enumerate(output.splitlines()):
    713         if idx:

/usr/local/lib/python2.7/dist-packages/pandas/core/base.pyc in __repr__(self)
     62         Yields Bytestring in Py2, Unicode String in py3.
     63         """
---> 64         return str(self)
     65 
     66 

/usr/local/lib/python2.7/dist-packages/pandas/core/base.pyc in __str__(self)
     42         if compat.PY3:
     43             return self.__unicode__()
---> 44         return self.__bytes__()
     45 
     46     def __bytes__(self):

/usr/local/lib/python2.7/dist-packages/pandas/core/base.pyc in __bytes__(self)
     54 
     55         encoding = get_option("display.encoding")
---> 56         return self.__unicode__().encode(encoding, 'replace')
     57 
     58     def __repr__(self):

/usr/local/lib/python2.7/dist-packages/pandas/core/index.pyc in __unicode__(self)
    396         """
    397         klass = self.__class__.__name__
--> 398         data = self._format_data()
    399         attrs = self._format_attrs()
    400         space = self._format_space()

/usr/local/lib/python2.7/dist-packages/pandas/core/index.pyc in _format_data(self)
    473             summary = '[%s], ' % first
    474         elif n == 2:
--> 475             first = formatter(self[0])
    476             last = formatter(self[-1])
    477             summary = '[%s, %s], ' % (first, last)

/usr/local/lib/python2.7/dist-packages/pandas/tseries/period.pyc in __getitem__(self, key)
    704         if np.isscalar(key):
    705             val = getitem(key)
--> 706             return Period(ordinal=val, freq=self.freq)
    707         else:
    708             if com.is_bool_indexer(key):

das/src/period.pyx in pandas._period.Period.__init__ (pandas/src/period.c:9740)()

ValueError: Ordinal must be an integer
```

Any ideas? Cheers

you need to call

```
In [4]: two_periods_index=period_index._shallow_copy(period_index[5:7].values)

In [5]: two_periods_index
Out[5]: PeriodIndex(['2015-01-08', '2015-01-09'], dtype='int64', freq='B')
```

`_shallow_copy` is expecting the 'base' form of an object, e.g. the actual values themselves (a ndarray with the dtype set correctly). It does not do any inference. You can update the doc-string if you'd like.

FYI I'm working on this, hit a couple of roadblocks, and will get something in over the next few days

After some digging, the broader issue for me is that every `Index` subtype can create itself by calling the `Index` constructor on its values (i.e. `pd.Index(self.values, **self._get_attributes_dict())`) **except** for `PeriodIndex`.

So I think either we:
1. Use `_shallow_copy` or something similar to recreate Indexes from values, and stop using the `Index` constructor 
2. Write separate implementations of a bunch of methods for `PeriodIndex`
3. Change the way `PeriodIndex` works so that you can build itself from its values and the `Index` constructor

I had started going down (1), but want to check in and see what people thoughts was the wisest path. And it seems like some policy for the best way would be helpful, as I see other PRs going through that are taking a different route (e.g. #10638 is doing (2) I think)

@maximilianr `PeriodIndex` currently doesn't conform as well as the other Indexes to the way things are done (e.g. using inherited methods and such). So the fixes  you are doing are ok. So 3) is actually the way to go.

#10638 is doing a specific change to datetimelike Indexes. (it was not supposed to be specifically on PeriodIndex).

That's the ideal, use as much of the super classes machinery as possible.

@jreback OK great. Just to confirm what you're saying - that I should continue doing (1) for now, _and_ that in time we should do (3)? 

what have done so far looks good
you may need to fix the constructor to use _shallow_copy (see what was done for Datetimesindex)

OK, I don't completely get you on your final comment. 
Regardless I'll finish this one and we can go from there

@jreback ready to go
Could use some oversight on the `append` usage - I wonder if there's a performance impact - but it does work, and centralizes the logic in that method so we can improve it from here

Yeah, using append twice means that we copy the index data twice, instead of only once. But it's unlikely anyone is using insert in performance sensitive code, anyways, so I'm not too concerned here.

Ok I squashed your commits together and added one here: https://github.com/jreback/pandas/tree/SixtyCapital-index-subtype

So pick this up and should be good to go.

was a bit more involved but I think got most of the wonkiness out.

As an aside (after this tests and passes), I _think_ this might address (maybe need some changes, but if you would write some tests and see)

for 

#9462 
#10033
and several issues in #9965

so `PeriodIndex` is a bit different from the other index types in that it cannot be inferred from the values themselves; and the dtype doesn't help (it just looks like an `Int64Index`). Where's almost all of the indexes CAN be inferred, or have explict construction (e.g. `CategoricalIndex`). 

Great! Thanks.
I'll go through this weekend and pull it all together

@jreback made a couple of tweaks - nothing major. 
Good to go, I think. 
Happy to squash our commits together if you prefer?
Will have a look at those other issues next

the commit you added does a lot of unnecessary things
what exactly are you trying to fix? 

OK, the names were breaking with the Categorical indexes - but seems like in the meantime we removed the names! I'll adjust and come back to you.

For the full explanation: specifically, in `test_indexing`: 

``` python
   4645         result = self.df2.reindex(pd.Categorical(['a','d'],categories=cats))
   4646         expected = DataFrame({'A' : [0,1,5,np.nan],
   4647                               'B' : Series(list('aaad')).astype('category',categories=cats) }).set_index('B')
```

```
ipdb> result.index
CategoricalIndex(['a', 'a', 'a', nan], categories=['c', 'a', 'b', 'e'], ordered=False, dtype='category')
ipdb> expected.index
CategoricalIndex(['a', 'a', 'a', nan], categories=['c', 'a', 'b', 'e'], ordered=False, name='B', dtype='category')
```

...and now, even if we have 
`4645         result = self.df2.reindex(pd.Categorical(['a','d'],categories=cats, name='B'))`, the name doesn't propagate, I think because a recent commit removed the name attribute from `Categorical`s

@jreback OK, ready to go at last - let me know if any concerns about the lack of testing names in the categorical reindexing though.

@maximilianr 

you must have something funny going on. The tests are fine as IS (and the names DO propogate across the reindex).

@jreback Sorry - I'm sure what was going on there. Seems I wasted a fair amount of time! Reverted and it works great now.

merged https://github.com/pydata/pandas/commit/06a4d9130106c968dacbaeb6103e22cf8abb40d8

thanks!