Fix interpolate -limit Add interpolate limit_direction='inside'
State: closed

 - [ x] closes #16282 and #16284
 - [x ] tests added / passed
 - [ x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [ x] whatsnew entry


Labels:

Comments:
pls add tests for any changes.
Sorry for the oversight. I will get the tests added. I also realized I
probably should have done two separate pulls.  On for the fix and one for
the enhancement.

Also, Baurzhan suggested a change from limit_direction='inside' to
surrounded.

Any thoughts on this? I'm thinking 'inside' sounds more like a direction,
but I will make it whatever.


On May 10, 2017 7:15 AM, "Jeff Reback" <notifications@github.com> wrote:

> pls add tests for any changes.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/16307#issuecomment-300451267>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/APZUQdS16yqKS4i86LHkyuQ9djmR5tR0ks5r4ZxDgaJpZM4NVrqz>
> .
>

I've added required tests for the limit direction feature.  I see them in my fork and I see them referenced in the pull request.

It should (I think) be ready to go, but I'm not sure if I need to do anything to update the pull.  If I've missed anything else, just let me know.  This is my first contribution, so please bear with me.

Thanks
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=h1) Report
> Merging [#16307](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/a6fcec6c1a8ef5fe1079cfc95e44bea9d76aa51a?src=pr&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16307/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #16307      +/-   ##
==========================================
+ Coverage   90.38%   90.41%   +0.02%     
==========================================
  Files         161      161              
  Lines       50999    51001       +2     
==========================================
+ Hits        46096    46111      +15     
+ Misses       4903     4890      -13
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.24% <100%> (+0.02%)` | :arrow_up: |
| #single | `40.19% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/missing.py](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvbWlzc2luZy5weQ==) | `84.37% <100%> (+0.09%)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.33% <0%> (+0.09%)` | :arrow_up: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.05% <0%> (+1.81%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=footer). Last update [a6fcec6...aa92985](https://codecov.io/gh/pandas-dev/pandas/pull/16307?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Hi Guys,

Looks like I got all checks passed, coverage increased with new tests, I got my spelling error fixed, and I currently have no conflicts with the base branch.

May I get a review on this?  It is actually a fairly simple change.  I just had to learn the process for submitting.

Thanks!
Hi Tom,

Please help me understand your comment.  There are currently two parameters in use:
limit=n  This is the NUMBER of missing values to fill in a given direction.
limit_direction='param' is an already existing parameter indicating the direction in which values should be filled.  Specifically, to the already existing values, I added a new direction of 'inside'.
- **forward** - fill forward including values past the last non-NaN
- **backward** - fill backward including values before the last non-NaN
- **both** - fill both directions including values before and after the last non-NaN
- ADD: _**inside**_ - fill values that are surrounded by non-NaN (will only fill missing values that are surrounded by non-NaN and thus will not expand size of the series).

The new direction 'inside' fits with the other existing directions.  Please help me understand the need to add a new parameter.

Thanks!
Your test kind of shows it (`s.interpolate(method='linear', limit=1, limit_direction='inside'):

```python
s = Series([np.nan, 1, 3, np.nan, np.nan, np.nan, 11, np.nan])
expected = Series([np.nan, 1, 3, 5, np.nan, 9, 11, np.nan])
```

It looks like that's a `limit_direction="both"`, but without extrapolate. You could imagine a case where the user wants to only fill forward (`limit_direction="forward"`) but not extrapolate, so their expected is

```python
expected = Series([np.nan, 1, 3, 5, np.nan, np.nan, 11, np.nan])
#                                           ^ this one changed to still be nan
```

Basically, extrapolation is different that interpolation ;)
Ah... right.  Yes, I agree with you.  My key issue was actually with the fact that we are not differentiating between extrapolating and interpolating.  I was fixing one case, but a general solution would cover all the cases.

By default scypi interpolates, and by default pandas interpolates AND extrapolates.

Perhaps:
limit_type=None (default to interpolate and extrapolate as it is today)
limit_type=interpolate (only fills inside values)
limit_type=extrapolate (only fills outside values)

Or better yet... maybe:
interpolate=True (by default... False to turn it off)
extrapolate=True (by default... False to turn it off)

Since I'm in the middle of this, I'd be happy to do this if you guys want it done.  Just let me know what the core contributors decide.

In the mean time, I will revert my code to master and just apply code required for the limit=n bug fix (I should not have combined these in the first place), then submit changes again.

Thanks!
@WBare let's move the extrapolate discussion back to https://github.com/pandas-dev/pandas/issues/16284