CLN: Series.asof uses reindex GH10343
State: closed

closes #10343 as discussed in #10345 , changed `Series.asof` to use `reindex` internally, and remove the unused leftovers from index classes. All existing tests in test_series.py:TestSeries.test_asof pass with the new implementation.


Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Clean

Comments:
nice work!

I'd like to see this put in `core/base.py/IndexOpsMixIn` as this mixes into `Series` so would be no change there, and could replace the `Index.asof` as well.

Tests should then move to `tests/test_base.py`

@jreback Ok, I'll see what I can do.

can you update according to comments

@jreback Yup, will get it done this week, sorry about the delay.

So I'm having a little difficulty combining this with `Index.asof` in the mixin, `Index.asof` seems to support some odd partial string matching for a `DatetimeIndex`, in particular (from `test_index.py:TestIndex.test_asof_datetime_partial`:

```
ipdb> idx
DatetimeIndex(['2010-01-31', '2010-02-28'], dtype='datetime64[ns]', freq='M', tz=None)
ipdb> idx.asof('2010-02')
Timestamp('2010-02-28 00:00:00', offset='M')
```

I don't think it's possible to get that result using reindex, (and I must admit I'm a little confused as to the use case where this is the desired behavior). 

I can of course still move the `Series` and `Index` `asof` implementation into a mixin with an `isinstance` check, but there isn't really much common between them at present.

Incidently, I was unable to find anywhere outside its own tests where `Index.asof` is used in the code base.

Where is the code that is creating that oddness? Happy to have a look. I couldn't find it in `DatetimeIndex`

@maximilianr Far as I've been able to figure out the call chain, it goes `Index.asof` -> `DatetimeIndex.get_loc` -> `DatetimeIndex._get_string_slice`.

Presumably the intended behavior since it's explicitly tested for (see `test_asof_datetime_partial` in `test_index.py`). It is however somewhat jarring since the result from passing a string and passing a timestamp generated from that string differ:

```
In [6]: idx
Out[6]: DatetimeIndex(['2010-01-31', '2010-02-28'], dtype='datetime64[ns]', freq='M', tz=None)

In [7]: idx.asof('2010-02')
Out[7]: Timestamp('2010-02-28 00:00:00', offset='M')

In [8]: idx.asof(pd.Timestamp('2010-02'))
Out[8]: Timestamp('2010-01-31 00:00:00', offset='M')
```

@bwillers hmm, I don't think `.asof` should accept a non-qualified date, e.g. it must be the same freq as the dates or raise.

@jreback I think its a valid asof question ("what's the most recent value before or on 2010-02-01") but the fact that calling with a string returns a backfilled rather than forward filled value seems wrong. But then, there's an explicit test requiring this, so it seems likely this was added for some particular purpose I'm unaware of.

See here for discussion on asof's handling of partial string labels: https://github.com/pydata/pandas/pull/9258#issuecomment-73624278

@bwillers I can't understand how that example is intended behavior. Do you (anyone?) know why that might be intended behavior?

I think it's fine to drop the functionality of string coercion. If it didn't add material complication I would vote to keep it (i.e. whatever you pass into `__getitem__` would be permissible), but not a strong view.

I can have a look at this in more detail this w/e if helpful. 

@maximilianr I do not know, I only discovered it a few days ago. Any insight is appreciated.

closing, pls reopen if you can fix according to comments