Allow interpolate() to fill backwards as well as forwards
State: closed

closes #9218
closes #10420 

**Edit: changed approach, see the rest of the thread**

Currently, interpolate() has a "limit" kwarg that, when set to n,
prevents interpolated values from propagating more than n rows forward.

This change adds a "backward_limit" kwarg that, when set to n, prevents
interpolated values from propagating more than n rows backward.

The behavior prior to this change is as though "backward_limit" existed
but was always set to 0. That is, interpolated values never filled in
NaNs immediately before a non-NaN value (unless those same NaNs happened
to follow a different, non-NaN value within "limit" rows).

In this change the "backward_limit" kwarg has no effect if "limit" is
not specified (i.e., None).


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

Comments:
I also have a local change to the documentation that adds an example of using the new kwarg, but I'd like to verify first that people think the behavior implemented here is good.

From an API perspective, it would be nice if `backward_limit` was treated the same way as `limit`. Also, maybe we want to deprecate the existing `limit` keyword argument in favor of the more explicit `forward_limit`?

I would rather see a `method='forward|backward'` (or for compat, `ffill|bfill`) to be like `.fillna`. Then this becomes unambiguous.

Thanks for the feedback!

@jreback if there's a forward/backward flag (or ffill/bfill), then the interpolation can only fill in one direction at a time? So I would have to do two interpolations to fill values around non-NaNs? (This is currently the case for fillna as well, just confirming that's what you have in mind.)

It would be nice if I could fill "forward", "backward", or "around" with just one call, since I can currently use your suggestion from #9218 and just reverse the sequence and interpolate (or fill) if I'm going to interpolate twice anyway.

@shoyer I agree that it would be nice to respect passing the "backward_limit" kwarg only ... this was just my first attempt to put some code in place. It seems like we ought to work out this forward/backward/around issue before getting too detailed on the backward_limit.

`around` would be ok too. I just want to limit the api differences with other routines.

Yes, totally agree, api consistency is a good thing!

I like the idea of adding `method=forward|backward|around` and then just using `limit` in whatever is the indicated direction. But `method` is already used by `interpolate` to indicate which type of interpolation to use (`linear`, `quadratic` etc.)

What if `limit` accepted an int or a pair of ints? If it's an int it would just apply like it currently does, if a list/tuple it would contain (backward, forward) limits? Perhaps too different from existing api conventions?

Sorry for the noise, I got my branch in a weird state and did a hard reset.

I changed the code so that limit accepts either an int (preserves current behavior) or a tuple of (backward, forward) ints.

Any comments on this version of the change?

@shoyer what you do you think?

To me, `limit` does not seem like something that should be inherently directional. So if we were starting over from scratch, I would make it apply in both directions.

That said, we are here where we are today, and we do want to make some effort to preserve backwards compatibility.

I'm not a really big fan of using tuples instead of another keyword argument. It seems a little awkward and not so Pythonic. I'm also not sure if there are legitimate use cases for limiting the interpolation only in one direction (maybe?).

If we don't care about asymmetrical limits, we could add a new keyword argument `symmetric_limit`. For now, the default would be `False`, but would also issue a deprecation warning. In some future version of pandas, we switch the default to `True`. Then, we eventually remove it entirely.

@shoyer I get what you're saying about using a tuple, it's a bit odd considering the way the rest of the pandas api works (at least what I've seen of it).

If we want to add a new kwarg, it could be something categorical like `limit_direction='forward'|'backward'|'both'`?

I'd also be fine with your `symmetric_limit` idea, too.

"If we want to add a new kwarg, it could be something categorical like `limit_direction='forward'|'backward'|'both'?`"

This seems like a very good way to me.  You can preserve backwards compatibility by setting the default to "forward" which also makes it obvious to a new user in the documentation why interpolate is only extrapolating in one direction (and how to change that).

It's also consistent with `fillna` behavior aside from the necessary use of a keyword other than 'method'.

FWIW, I think allowing asymmetric behavior is really important overall for this command.  I don't think having different limits is all that important though.  That is, a user will often want to interpolate forwards or backwards (but not both).  In cases where the user wants to go in both directions I'd expect a symmetric limit is fine.  I.e. I think it would be fairly uncommon to want to set a limit of, say, 2 in one direction and 5 in the other.  And on those rare occasions, I think just running 2 separate interpolations is not that onerous.

I would be +1 on adding `limit_direction`.

I've updated the change in my local branch to add a `limit_direction` keyword argument. It defaults to `'forward'` which preserves the existing behavior. If it's set to `'backward'` the limit applies in a backward direction, and if it's set to some other value the `limit` applies in both directions. (I use `'both'` in the test and the docs.)

The change does not change the current behavior at all when `limit` is falsey. So in particular if you want to fill all the values at the beginning of a series (e.g. #10420) you need to provide a nonzero `limit` and also `limit_direction='backward'` or `limit_direction='both'`. This matches the current default behavior and to me seems like an ok thing to ask of people who really want to fill in all the `NaN` values in their series.

I'm not such a big fan of `extrapolate` since in my brain extrapolation only refers to the extreme beginning or end of a series, i.e., when there's not another valid data point to aim for.

I thought a bit about a `'none'` or something like that, but then why would you even call `interpolate` in the first place?

@lmjohns3   Sorry about the `extrapolation` comment, I have since deleted it after I realized the `limit` arg is applied to both interpolations and extrapolations, so it didn't really make any sense.

Regarding extrapolation, I think things are still a little strange in this sense.  If you do the simplest version `df.interpolate()` you really just get an interpolation followed by `ffill()`.  But if you do `df.interpolate(method='spline',order=1)` you'll get a true extrapolation.  (see below)

But I don't mean to further complicate the current issue.  One step at a time and I think this is a very good change you are making.

```
df=pd.DataFrame( [ 0, np.nan, 2, np.nan] )

df.interpolate()
Out[890]: 
   0
0  0
1  1
2  2
3  2

df.interpolate(method='spline',order=1)
Out[891]: 
   0
0  0
1  1
2  2
3  3
```

@johne13 This is a good point you bring up, and might require a bit more thought/testing (but I'm not sure!). It looks like interpolate() actually is a sort of wrapper for several different interpolation methods. The change as it currently stands only applies to the variant where `method` is specified. I'm not sure how much that limits the change, or if that would be confusing to people.

I just checked further and the code is a bit wacky, but it looks like this change should be ok.

There are basically two branches inside the `Block.interpolate` method in `internals.py`. If `method` is one of ffill, bfill, pad, backfill, or nearest, then interpolate basically does the same thing as fillna (i.e., it doesn't actually do an interpolation in the function-fitting sense). But if `method` is one of the scipy interpolate methods, then it does a function-fitting interpolation.

There's also a `CategoricalBlock.interpolate` method, and a `SparseBlock.interpolate` method. Both of these appear only to fill, not to interpolate.

The interpolate methods are called from a number of different places, but the main one I'm after is `NDFrame.interpolate` in `generic.py` (both `Series` and `DataFrame` are subclasses). The default `method` here is `'linear'` so it seems like this change should cover the right cases.

If anyone is more familiar with the API and this raises any red flags let me know!

Any further comments on this PR?

Thanks for the comments!

I've addressed the test issues, and I've tried to update the docs in the way that I think you were describing.

I made a new "interpolation limits" section in the "missing_data.rst" file, but I'm unsure if it's kosher -- I used ^^^^^ for the headline level, but there are no other headlines in any of the other docs that use that level. Maybe I should just make the section header bold?

Likewise in the whatsnew file I added a small example, but if anything about the formatting etc. looks weird to you let me know.

Any further comments on this? And suggestions for the ^^^^ header level?

@TomAugspurger can you have a look

@lmjohns3 need to update the doc-string for `.interpolate` (in core/generic.py)

I just added the `ser.interpolate(limit=1)` example in the doc, and updated the interpolate docstring.

(Also: So far I've been committing all these incremental changes as fixups to the original commit, so that there will be just one commit with this change after merging. Let me know if it's better to keep them as individual commits in my fork for some reason.)

@lmjohns3 this is what I do, others just add everything in their own commit.

Its sort of up to you when deving. Eventually I'll have you squash if you didn't, so this is sort of a continuous squash.

lgtm.

@jorisvandenbossche 
@shoyer 

@lmjohns3 need validation on the `limit_direction` that it can only be `forward|backward|both` (and tests for same).

OK, I've made a number of updates to the patch that address the comments above.
- limit_direction is limited to 'forward', 'backward', 'both', or 'symmetric' ('both' and 'symmetric' do the same thing), but can be passed in any combination of uppercase/lowercase. raises ValueError if not one of these values
- tests for case/validity of limit_direction kwarg, filling values backward to beginning of series, and different limit_direction cases

In dealing with the case of filling a series backward to the beginning, I had to muck around with the interpolate_1d function a good bit more than before. This makes the diff quite ugly ... I'd be happier with it if I could check in a separate PR first that does the code-rearranging part of things. Any opinions on how to do that? Or is everyone comfortable with an ugly change that is nonetheless pretty well tested?

Thanks for everyone's feedback on this by the way!

Redundant "both" and "symmetric" arguments seems like a bad idea. I would
pick one and stick to it (probably "both").

On Tue, Aug 18, 2015 at 11:11 AM, Leif Johnson notifications@github.com
wrote:

> OK, I've made a number of updates to the patch that address the comments
> above.
> - limit_direction is limited to 'forward', 'backward', 'both', or
>   'symmetric' ('both' and 'symmetric' do the same thing), but can be passed
>   in any combination of uppercase/lowercase. raises ValueError if not one of
>   these values
> - tests for case/validity of limit_direction kwarg, filling values
>   backward to beginning of series, and different limit_direction cases
> 
> In dealing with the case of filling a series backward to the beginning, I
> had to muck around with the interpolate_1d function a good bit more than
> before. This makes the diff quite ugly ... I'd be happier with it if I
> could check in a separate PR first that does the code-rearranging part of
> things. Any opinions on how to do that? Or is everyone comfortable with an
> ugly change that is nonetheless pretty well tested?
> 
> Thanks for everyone's feedback on this by the way!
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/pydata/pandas/pull/10691#issuecomment-132303159.

Done -- it's just 'both' now.

I was also a bit surprised by this change, because it turns out that both `numpy.interp` and `scipy.interp1d` resort to "filling" behavior for extrapolations -- they just fill the extrapolated index values with the closest observed value.

To get a true extrapolation you'd have to use something like this:

```
ser.interpolate(method='spline', order=1, limit=10000, limit_direction='both')
```

But at least it's possible now!