Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG/ER: Stricter testing of 'monotocity' when reindexing with ffill (GH4483) #4642

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions doc/source/basics.rst
Expand Up @@ -724,16 +724,16 @@ We illustrate these fill methods on a simple TimeSeries:
ts2.reindex(ts.index, method='ffill')
ts2.reindex(ts.index, method='bfill')

Note these methods require that the indexes are **order increasing**.

Note the same result could have been achieved using :ref:`fillna
<missing_data.fillna>`:

.. ipython:: python

ts2.reindex(ts.index).fillna(method='ffill')

Note these methods generally assume that the indexes are **sorted**. They may
be modified in the future to be a bit more flexible but as time series data is
ordered most of the time anyway, this has not been a major priority.
Note that this method does not check the order of the index.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add: that this will raise a ValueError will be raised if the index is not monotonic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now fixed.


.. _basics.drop:

Expand Down
3 changes: 3 additions & 0 deletions doc/source/missing_data.rst
Expand Up @@ -205,6 +205,9 @@ To remind you, these are the available filling methods:
With time series data, using pad/ffill is extremely common so that the "last
known value" is available at every time point.

The ``ffill()`` function is equivalent to ``fillna(method='ffill')``
and ``bfill()`` is equivalent to ``fillna(method='bfill')``

.. _missing_data.dropna:

Dropping axis labels with missing data: dropna
Expand Down
2 changes: 2 additions & 0 deletions doc/source/release.rst
Expand Up @@ -62,6 +62,8 @@ pandas 0.13

**API Changes**

- ``DataFrame.reindex()`` and forward/backward filling now raises ValueError
if either index is not monotonic (:issue: `4483`, :issue: `4484`).
- ``pandas`` now is Python 2/3 compatible without the need for 2to3 thanks to
@jtratner. As a result, pandas now uses iterators more extensively. This
also led to the introduction of substantive parts of the Benjamin
Expand Down
8 changes: 4 additions & 4 deletions pandas/core/index.py
Expand Up @@ -905,12 +905,12 @@ def get_indexer(self, target, method=None, limit=None):
' valued Index objects')

if method == 'pad':
if not self.is_monotonic:
raise AssertionError('Must be monotonic for forward fill')
if not self.is_monotonic or not target.is_monotonic:
raise ValueError('Must be monotonic for forward fill')
indexer = self._engine.get_pad_indexer(target.values, limit)
elif method == 'backfill':
if not self.is_monotonic:
raise AssertionError('Must be monotonic for backward fill')
if not self.is_monotonic or not target.is_monotonic:
raise ValueError('Must be monotonic for backward fill')
indexer = self._engine.get_backfill_indexer(target.values, limit)
elif method is None:
indexer = self._engine.get_indexer(target.values)
Expand Down
23 changes: 23 additions & 0 deletions pandas/tests/test_frame.py
Expand Up @@ -1641,6 +1641,29 @@ def test_nested_exception(self):
except Exception as e:
self.assertNotEqual(type(e), UnboundLocalError)

def test_reverse_reindex_ffill_raises(self):
dr = pd.date_range('2013-08-01', periods=6, freq='B')
data = np.random.randn(6,1)
df = pd.DataFrame(data, index=dr, columns=list('A'))
df['A'][3] = np.nan
df_rev = pd.DataFrame(data, index=dr[::-1], columns=list('A'))
# Reverse index is not 'monotonic'
self.assertRaises(ValueError, df_rev.reindex, df.index, method='pad')
self.assertRaises(ValueError, df_rev.reindex, df.index, method='ffill')
self.assertRaises(ValueError, df_rev.reindex, df.index, method='bfill')

def test_reversed_reindex_ffill_raises(self):
dr = pd.date_range('2013-08-01', periods=6, freq='B')
data = np.random.randn(6,1)
df = pd.DataFrame(data, index=dr, columns=list('A'))
df['A'][3] = np.nan
df = pd.DataFrame(data, index=dr, columns=list('A'))
# Reversed reindex is not 'monotonic'
self.assertRaises(ValueError, df.reindex, dr[::-1], method='pad')
self.assertRaises(ValueError, df.reindex, dr[::-1], method='ffill')
self.assertRaises(ValueError, df.reindex, dr[::-1], method='bfill')


_seriesd = tm.getSeriesData()
_tsd = tm.getTimeSeriesData()

Expand Down
2 changes: 1 addition & 1 deletion pandas/tseries/tests/test_timeseries.py
Expand Up @@ -546,7 +546,7 @@ def test_pad_require_monotonicity(self):

rng2 = rng[::2][::-1]

self.assertRaises(AssertionError, rng2.get_indexer, rng,
self.assertRaises(ValueError, rng2.get_indexer, rng,
method='pad')

def test_frame_ctor_datetime64_column(self):
Expand Down