CLN: refactor Series.reindex to core/generic
State: closed

closes #4604, #3317, #4618

Incorrect results

```
In [1]: s = pd.Series([1,2,3,4,5], index=['a', 'b', 'c', 'd', 'e'])
In [2]: s.reindex(['a', 'g', 'c', 'f'], method='ffill')
Out[2]: 
a     1
g     5
c   NaN
f     5
dtype: float64
```

Fixed (and happens to automatically cast as well back to the original dtype)
(this was broken in 0.12)

```
In [2]: s.reindex(['a', 'g', 'c', 'f'], method='ffill')
Out[2]: 
a    1
g    1
c    3
f    3
dtype: int64
```

This does NOT automatically downcast here as this is a major
performance bottleneck if we did (this is the same result as in 0.12)

```
In [3]: >>> s.reindex(['a', 'g', 'c', 'f']).ffill()
Out[3]: 
a    1
g    1
c    3
f    3
dtype: float64
```

You can force downcasting by passing 'infer' if you want
(this option was not available in 0.12 for Series)

```
In [4]: >>> s.reindex(['a', 'g', 'c', 'f']).ffill(downcast='infer')
Out[4]: 
a    1
g    1
c    3
f    3
dtype: int64
```

TST: GH4604, reindexing with a method of 'ffill' gives incorrect results

BUG/CLN: (GH4604) Refactor Series.reindex to core/generic.py allow method= in reindexing
  on a Series to work

API/CLN: GH4604 Infer and downcast dtype if appropriate on ffill/bfill
  this is for consistency when doing: df.reindex().ffill() and df.reindex(method='ffill')

CLN: allow backfill/pad/interpolate to operate on integers (by float conversion)
     provide downcasting back to original dtype where needed core.internals.interpolate

ENH: provide core.index.identical method to compare values and attributes similar to .equals

API: changed back to pre-GH3482 where a reindex with no args will by default copy


Labels:

Comments:
cc @jtratner

I had to create a core.index.identical method (similar to .equals) but also compares attributes (e.g. name)

we don't have anything like this, right?

do you know python calls for `x is y` test? (aside from `__eq__` or is that it?

There's an issue for that isn't there? Comparing indexes including meta data 

prob is
can u link if there is (what I did prob wont solve it)

could only find https://github.com/pydata/pandas/issues/3317, with a just passing mention....should i open a new issue for it? really just a decision....i remember preserving `name` broke a few tests. can't remember if it was a bunch or just a few though...i might have it in a branch

 `x is y` tests for object identity, so it won't call any methods on the object. It's not (and shouldn't be) guaranteed that `Index` will remain the same object after transformations, even though it doesn't require copying any of the index attributes.

Example of `is` checks:

``` python
lst = [1, 2, 3]
lst2 = list(lst)
assert not (lst is lst2)
```

You could potentially add a `np.may_share_memory` check (but only for non-MultiIndex).

I'm not totally clear on what you're looking for here.  I don't think it's a good idea to have identical test for object identity (though, granted, there is _some_ overhead with views of MultiIndex, but not much [approx space complexity O(n+m) where n and m are length of the set of levels and length of the set of labels]).

I'm wrong on space complexity of MI view - it's whatever space python needs to make a list pointing to all the lists of levels and a list pointing to all the lists of labels. (so a MI with 2 sets of levels and 2 lists of labels costs two lists of length  2 to shallow copy) very minimal. 

@jtratner 

its just a very simple issue

if you are reindexing with an index that is _identical_ (and copy is not set), then just return the object

but this HAS to account for an _equals_ index, but with a different name for example
(which relates to the semi-mutable index)

makes sense?

```
In [6]: index = pd.Index(list(range(5)))

In [7]: index2 = pd.Index(list(range(5))).copy(name='foo')

In [8]: index.equals(index)
Out[8]: True

In [9]: index.identical(index)
Out[9]: True

In [10]: index.identical(index2)
Out[10]: False

In [11]: index.equals(index2)
Out[11]: True

In [12]: index.identical(index2)
Out[12]: False
```

```
In [13]: df = DataFrame(randn(5,2),index=index)

In [17]: df.reindex(index2).index.identical(index2)
Out[17]: True

In [18]: df.reindex(index2).index.equals(index2)
Out[18]: True

In [20]: df.reindex(index2).index.identical(index)
Out[20]: False

In [21]: df.reindex(index2).index.equals(index)
Out[21]: True

```

Does `identical` also test the `freq` attribute of the appropriate indexes?

yep freq/tz on datetime index, freq on period

on MI, levels are already tested (and I think labels) via equals, so it just tests name, 

:+1: 

what if `identical` replaces `__eq__` ?

so all current code will work, but then true equality is tested by `index == index2` or is that confusing?

that will break tests... plus that might be kind of a nasty surprise to people who don't religiously keep track of `tz` and `freq`

fair enough...

for the record i would like that, but too much back-incompat. would actually fix #3317 i believe

Oh okay, I wasn't totally clear on your question. Looks good :)

@jtratner @cpcloud pls give this a once over..thxs

Will look over in a minute.

@jreback this doesn't quite work for MultiIndex yet. Is it supposed to?

E.g.

``` python
mi = pd.util.testing.makeCustomDataframe(5, 5).stack().index
assert mi.identical(mi)
other = mi.from_tuples(tuple(mi))
assert not mi.identical(other), "Failed - other does not have same names"
```

On a related note - for some reason, MultIIndex appears to have a `name` attribute that returns `None`, which shadows the issue. (i.e., continuing previous)

``` python
In [16]: print(mi.name)
None

In [17]: print(mi.names)
FrozenList([u'R0', u'C0'])

In [18]: print(other.name)
None

In [19]: print(other.names)
FrozenList([None, None])
```

Ah, it's because `Index` sets `name=None` explicitly. Is it worth adding a property to MultiIndex to `mi.name` fail?

I think there is prob some compatibility going on here

iow you need to know whether an object is an index or mi and then use name or names

@jtratner side issue - `rename` in index is broken for MultiIndex, and need some tests...thanks

@jreback the real problem is that rename and name should not exist for MultiIndex (they only make sense for `Index` objects).

That said, why can't you just set `_comparables = ['names']`?

`rename` does makes sense for MI, e.g.

`mi.set_names(['level_0','level_1'])`

?

@jtratner oh I did do that, I was trying to use `rename` in some tests; it breaks for MI....that's all (you can just use `set_names`, but the arguments are only built for a single name), and the `rename` for an `Index` should be able to take a list of a scalar (e.g. `i.rename('foo')` doesn't work, but `i.rename(['foo'])` does...(I know its a convenience function)....but ...

@jreback You're right, that's definitely broken. 

@jreback I'll put up a fix tomorrow. Sorry about that!

no...thank you for testing.....when I find something breaking...I realized that I didn't test it!