err#8038: Index.shift() gives confusing error message when no Datetim…
State: closed

…eIndex

closes #8038 


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
Is this the behavior you want though, vs. a clearer error message? This doesn't seem like it's shifting the index; it seems like it's adding 1...

``` python
        result = pd.Index(range(0,10,2)).shift(1)
        expected = pd.Index([1, 3, 5, 7, 9], dtype='int64')
```

@hamedhsn I don't this should even work on a non-datetimelike index as it doesn't make any sense. So we should prob raise a `TypeError`.

pls add a test and a whatsnew note in 0.17.1

@jreback 
1)why do I need to change tseries/base/DatetimeIndexOpsMixIn ? 
2)where should I add tests?

I indicated above. The `.shfit` in`core/index.py`applies to`Index/Int64Index`etc. These are not valid for shifting and so should raise`NotImplementedError`. Then make the actual change in`tseries/base/DatetimeIndexOpsMixIn`which is the base class for the datetimelikes (`DatetimeIndex,TimeDeltaIndex,PeriodIndex``) where shifting makes sense

sure, but there is no issue with DatetimeIndex using shift() with or without freq. that was my question.

the error happens for Index/Int64Index which i changed it to raise NotImplementedError. 
btw where do i have to add the related tests?

@hamedhsn you can add tests in `test_index` (though the timeseries ones are prob in `tseries/test_base`)

@jreback this also should be fine

@jreback @hamedhsn I think this behavior is confusing. I think there are some reasons to shift a non-DateTime-like index, but it shouldn't be done with a `freq`.

Do you guys think this is clear and expected behavior?

``` python
        idx = Index(range(5))   
        result = idx.shift(periods=2, freq=2)
        expected = pd.Index(range(4, 9))
        self.assert_index_equal(result, expected)
```

A version I could see being helpful is to shift by a number of items. i.e. this:

``` python
        idx = Index(range(5,10))   
        result = idx.shift(1)
        expected = pd.Index(range(6, 11))
        self.assert_index_equal(result, expected)
```

...although I'm more against the current implementation than I am fighting for this specific implementation.

As ever, open to being wrong though.

@maximilianr that would be a major change to add a freq to a non-timeseries Index. So wouldn't want to support that (here). 

@jreback That's exactly what I'm saying. This PR enables `freq` for shifting non-timeseries index (or am I woefully mistaken)? This is a copy paste from the new test code:

``` python
        idx = Index(range(5))   
        result = idx.shift(periods=2, freq=2)
        expected = pd.Index(range(4, 9))
        self.assert_index_equal(result, expected)
```

that's out of scope for this PR
would be unexpected as freq is not a property of an Index 

you are free to propose it but make an issue

I don't think I'm being clear. Your comment [here](https://github.com/pydata/pandas/pull/11211#discussion_r41131777) would fix the concern, but that hasn't been added. 
Or I'm likely missing something, so I'll leave it for now

@jreback is this implementation correct? let me know if I need to anything

a lot closer....still a couple of comments

@jreback applied your comments

one more comment. pls squash as well.

@jreback did that. let me know if I need to do anything else. Thanks

comments; needs a rebase

@jreback applied the comments

@jreback should be fine now hopefully

@hamedhsn if you would like to update according to comments would be great

@jreback I think I did, used self.create_index(), put that in DateTimelike. and for the other one moved that to based class in test_index.py and used self.create_index(). and rebased that.

@jreback added the other two tests under their own test classes

xref #11452 

@hamedhsn ok, tests look good (you just need to remove an error line and they would pass). pls add the other requested tests and good 2 go.

@jreback applied your comments.rebased it

merged via https://github.com/pydata/pandas/commit/a05eceba6c869217c5c6b8bf2bb57ff5cef8e681

thanks!

It seems I still have this issue see my comment in: #11452. I'm using pandas v0.19.2 via pip.