Repository: pandas-dev/pandas
Pull request: BUG: Interpolate limit=n GH16282
 - [x] closes #16282
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [x] whatsnew entry


Problem statement:
df.interpolate limit_direction= does not work without limit=x setting
the interpolate limit_direction setting should work even without a limit=x setting.

When interpolate is called without a limit=x setting, it always (apparently) uses the limit_direction setting default of 'forward' even when it is explicitly set to 'backward' or 'both'

The code below will show 3 DataFrames.

- The first shows the initial before interpolation.
- The second shows the interpolation done correctly in the backward direction.
- The third shows the dataframe done incorrectly in the 'forward' direction even though the limit_direction='backward' was used. 

The docs do not mention a linkage between the two settings, but if this is expected behavior (I hope not), it would be oh so nice to raise an exception like the existing error for a bad limit_direction setting.

Many thanks for all your work!

#### Example Code:

```python
import pandas as pd
import numpy as np

dfMain = pd.DataFrame({
    'a': [0, 1, np.NAN, 3, 4],
    'b': [np.NaN, 1, np.NaN, 3, 4],
    'c': [0 , 1, 2, 3, np.NaN]})

print("initial df")
print(dfMain)

print("backwards with limit=1 (correct result):")
print(dfMain.interpolate(limit_direction='backward', limit=1))

print("backwards no longer works when limit is removed (it is going forward):")
print(dfMain.interpolate(limit_direction='backward'))
```

#### Output of ``pd.show_versions()``

<details>
INSTALLED VERSIONS
------------------
commit: None
python: 3.6.0.final.0
python-bits: 64
OS: Linux
OS-release: 4.4.0-75-generic
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.19.2
nose: 1.3.7
pip: 9.0.1
setuptools: 34.4.1
Cython: 0.25.2
numpy: 1.12.1
scipy: 0.18.1
statsmodels: 0.6.1
xarray: None
IPython: 5.1.0
sphinx: 1.5.1
patsy: 0.4.1
dateutil: 2.6.0
pytz: 2016.10
blosc: None
bottleneck: 1.2.0
tables: 3.3.0
numexpr: 2.6.1
matplotlib: 2.0.0
openpyxl: 2.4.1
xlrd: 1.0.0
xlwt: 1.2.0
xlsxwriter: 0.9.6
lxml: 3.7.2
bs4: 4.5.3
html5lib: None
httplib2: None
apiclient: None
sqlalchemy: 1.1.5
pymysql: None
psycopg2: None
jinja2: 2.9.4
boto: 2.45.0
pandas_datareader: 0.2.1
None
initial df


</details>

Hints from issue discussion:
Thanks for the report, want to take a look at where it's going wrong? Maybe somewhere around https://github.com/pandas-dev/pandas/blob/9e57d915744623bef31170629bd1996eda3352cf/pandas/core/internals.py#L908
I've got a fix for this and the associated enhancement for limit_direction='inside' GH16284.

The issue was inside missing.py interpolate_1d

I have tested myself, and I'm running the test suite now, but since I'm not familiar with the code base, someone really should carefully review the code.  It is actually fairly simple, but I don't know if there are other dependencies.

Commit message: Test: Added small test for code coverage

Patch to review:
diff --git a/doc/source/whatsnew/v0.21.0.txt b/doc/source/whatsnew/v0.21.0.txt
index 36dffc3d3378b..aeb6204fd2229 100644
--- a/doc/source/whatsnew/v0.21.0.txt
+++ b/doc/source/whatsnew/v0.21.0.txt
@@ -106,6 +106,8 @@ Reshaping
 
 Numeric
 ^^^^^^^
+- DataFrame.interpolate was not respecting limit_direction when using the default limit=None (unlimited). Specifically, it would always use limit_direction='forward' even when limit_direction was set otherwise. Now default limit=None will work with other directions. :issue:`16282`
+
 
 
 
diff --git a/pandas/core/missing.py b/pandas/core/missing.py
index 3010348423340..51778684d68f5 100644
--- a/pandas/core/missing.py
+++ b/pandas/core/missing.py
@@ -160,35 +160,41 @@ def _interp_limit(invalid, fw_limit, bw_limit):
     start_nans = set(range(ys.first_valid_index()))
     end_nans = set(range(1 + ys.last_valid_index(), len(valid)))
 
-    # This is a list of the indexes in the series whose yvalue is currently
-    # NaN, but whose interpolated yvalue will be overwritten with NaN after
-    # computing the interpolation. For each index in this list, one of these
-    # conditions is true of the corresponding NaN in the yvalues:
+    # violate_limit is a list of the indexes in the series whose yvalue is
+    # currently NaN, and should still be NaN after the interpolation.
+    # Specifically:
     #
-    # a) It is one of a chain of NaNs at the beginning of the series, and
-    #    either limit is not specified or limit_direction is 'forward'.
-    # b) It is one of a chain of NaNs at the end of the series, and limit is
-    #    specified and limit_direction is 'backward' or 'both'.
-    # c) Limit is nonzero and it is further than limit from the nearest non-NaN
-    #    value (with respect to the limit_direction setting).
+    # If limit_direction='forward' or None then the list will contain NaNs at
+    # the beginning of the series, and NaNs that are more than 'limit' away
+    # from the prior non-NaN.
     #
-    # The default behavior is to fill forward with no limit, ignoring NaNs at
-    # the beginning (see issues #9218 and #10420)
-    violate_limit = sorted(start_nans)
-
-    if limit is not None:
-        if not is_integer(limit):
-            raise ValueError('Limit must be an integer')
-        if limit < 1:
-            raise ValueError('Limit must be greater than 0')
-        if limit_direction == 'forward':
-            violate_limit = sorted(start_nans | set(_interp_limit(invalid,
-                                                                  limit, 0)))
-        if limit_direction == 'backward':
-            violate_limit = sorted(end_nans | set(_interp_limit(invalid, 0,
-                                                                limit)))
-        if limit_direction == 'both':
-            violate_limit = sorted(_interp_limit(invalid, limit, limit))
+    # If limit_direction='backward' then the list will contain NaNs at
+    # the end of the series, and NaNs that are more than 'limit' away
+    # from the subsequent non-NaN.
+    #
+    # If limit_direction='both' then the list will contain NaNs that
+    # are more than 'limit' away from any non-NaN.
+    #
+    # If limit=None, then use default behavior of filling an unlimited number
+    # of NaNs in the direction specified by limit_direction
+
+    # default limit is unlimited GH #16282
+    if limit is None:
+        limit = len(xvalues)
+    elif not is_integer(limit):
+        raise ValueError('Limit must be an integer')
+    elif limit < 1:
+        raise ValueError('Limit must be greater than 0')
+
+    # each possible limit_direction
+    if limit_direction == 'forward':
+        violate_limit = sorted(start_nans |
+                               set(_interp_limit(invalid, limit, 0)))
+    elif limit_direction == 'backward':
+        violate_limit = sorted(end_nans |
+                               set(_interp_limit(invalid, 0, limit)))
+    elif limit_direction == 'both':
+        violate_limit = sorted(_interp_limit(invalid, limit, limit))
 
     xvalues = getattr(xvalues, 'values', xvalues)
     yvalues = getattr(yvalues, 'values', yvalues)
diff --git a/pandas/tests/series/test_missing.py b/pandas/tests/series/test_missing.py
index c52c41877d5c0..8e73c17684a16 100644
--- a/pandas/tests/series/test_missing.py
+++ b/pandas/tests/series/test_missing.py
@@ -931,6 +931,24 @@ def test_interp_limit_forward(self):
                                limit_direction='FORWARD')
         assert_series_equal(result, expected)
 
+    def test_interp_unlimited(self):
+        # these test are for issue #16282 default Limit=None is unlimited
+        s = Series([np.nan, 1., 3., np.nan, np.nan, np.nan, 11., np.nan])
+        expected = Series([1., 1., 3., 5., 7., 9., 11., 11.])
+        result = s.interpolate(method='linear',
+                               limit_direction='both')
+        assert_series_equal(result, expected)
+
+        expected = Series([np.nan, 1., 3., 5., 7., 9., 11., 11.])
+        result = s.interpolate(method='linear',
+                               limit_direction='forward')
+        assert_series_equal(result, expected)
+
+        expected = Series([1., 1., 3., 5., 7., 9., 11., np.nan])
+        result = s.interpolate(method='linear',
+                               limit_direction='backward')
+        assert_series_equal(result, expected)
+
     def test_interp_limit_bad_direction(self):
         s = Series([1, 3, np.nan, np.nan, np.nan, 11])
 
diff --git a/pandas/tests/test_common.py b/pandas/tests/test_common.py
index d7dbaccb87ee8..77ef535e08964 100644
--- a/pandas/tests/test_common.py
+++ b/pandas/tests/test_common.py
@@ -16,6 +16,7 @@ def test_mut_exclusive():
         com._mut_exclusive(a=1, b=2)
     assert com._mut_exclusive(a=1, b=None) == 1
     assert com._mut_exclusive(major=None, major_axis=None) is None
+    assert com._mut_exclusive(a=None, b=2) == 2
 
 
 def test_get_callable_name():


Code review comments:
diff_hunk: @@ -106,6 +106,8 @@ Reshaping
 
 Numeric
 ^^^^^^^
+- DataFrame.interpolate was not respecting limit_direction when using the default limit=None (unlimited). Specifically, it would always use limit_direction='forward' even when limit_direction was set otherwise. Now default limit=None will work with other directions. :issue:`16282`
path: doc/source/whatsnew/v0.21.0.txt
text: @user1:
you can move this to 0.20.0. Make much shorter. 

Bug in ``.interpolate()``, where ``limit_direction`` was not respected when ``limit=None`` (default) was passed (:issue:`16282`)

@author:
Hi Jeff @user1 ,  2 questions:

1) Can you give me a basic pointer on getting this into 0.20.0?  i.e. is that a new pull request?  Is there a way I can apply my branch to 0.20.0 (I'm just learning GitHub).

2) What is the proper way to ping when a pull request is ready for review (all green).  Just a comment?

Thanks!

@user2:
I think 0.20 was a typo. Probably mean 0.20.2. There's a `whatsnew/v0.20.2.txt`. You can move this section there (and make it a bit shorter. Typically just a sentence, and people can click on the link to the issue if they're interested in more).

And yeah, just a comment when you're ready for review, or when you notice that all the checkmarks at the bottom are green and it's ready for review.

@author:
Thanks @user2, I thought Jeff wanted me to somehow move the change to the 0.20 branch.  That change is in.  I will ping when green.