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

Fix Index __mul__-like ops with timedelta scalars #19333

Merged
merged 29 commits into from
Feb 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
fecc906
update whatsnew
jbrockmendel Jan 21, 2018
7e8d5ff
add rmul test to timedeltas
jbrockmendel Jan 21, 2018
d2423ca
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 21, 2018
1dd4ccf
handle Index * timedelta, with tests
jbrockmendel Jan 21, 2018
71811c8
whatsnew note
jbrockmendel Jan 21, 2018
4c2c74b
division tests, fix typos
jbrockmendel Jan 21, 2018
d45ba92
Fix typo
jbrockmendel Jan 21, 2018
4abd61a
restore whitespace
jbrockmendel Jan 21, 2018
11424b0
fill in GH issue
jbrockmendel Jan 21, 2018
ee48c8b
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 21, 2018
1c97489
whitespace fixup from merge
jbrockmendel Jan 21, 2018
23b7f44
arrange whatsnew notes
jbrockmendel Jan 22, 2018
086be08
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 22, 2018
6e04aba
comment, requested edits
jbrockmendel Jan 22, 2018
1012939
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 23, 2018
2fb3688
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 25, 2018
0ed9276
xfail RangeIndex test
jbrockmendel Jan 25, 2018
e798653
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 25, 2018
a04f970
fix pytest xfail problem
jbrockmendel Jan 25, 2018
878be31
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Jan 31, 2018
d62983d
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Feb 1, 2018
040b1e4
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Feb 2, 2018
d7dfba9
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Feb 5, 2018
a46acb9
remove xfail
jbrockmendel Feb 13, 2018
e6fa53b
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Feb 13, 2018
814e858
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Feb 21, 2018
080c06c
troubleshoot RangeIndex test failure
jbrockmendel Feb 21, 2018
22d155e
fixup reversed op
jbrockmendel Feb 21, 2018
9e3dec4
Merge branch 'master' of https://github.com/pandas-dev/pandas into id…
jbrockmendel Feb 21, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/source/whatsnew/v0.23.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,7 @@ Timedelta
- Bug in :func:`Timedelta.__floordiv__`, :func:`Timedelta.__rfloordiv__` where operating with a ``Tick`` object would raise a ``TypeError`` instead of returning a numeric value (:issue:`19738`)
- Bug in :func:`Period.asfreq` where periods near ``datetime(1, 1, 1)`` could be converted incorrectly (:issue:`19643`)
- Bug in :func:`Timedelta.total_seconds()` causing precision errors i.e. ``Timedelta('30S').total_seconds()==30.000000000000004`` (:issue:`19458`)
- Multiplication of :class:`TimedeltaIndex` by ``TimedeltaIndex`` will now raise ``TypeError`` instead of raising ``ValueError`` in cases of length mis-match (:issue`19333`)
-

Timezones
Expand Down Expand Up @@ -778,6 +779,7 @@ Numeric
- Bug in the :class:`DataFrame` constructor in which data containing very large positive or very large negative numbers was causing ``OverflowError`` (:issue:`18584`)
- Bug in :class:`Index` constructor with ``dtype='uint64'`` where int-like floats were not coerced to :class:`UInt64Index` (:issue:`18400`)
- Bug in :class:`DataFrame` flex arithmetic (e.g. ``df.add(other, fill_value=foo)``) with a ``fill_value`` other than ``None`` failed to raise ``NotImplementedError`` in corner cases where either the frame or ``other`` has length zero (:issue:`19522`)
- Multiplication and division of numeric-dtyped :class:`Index` objects with timedelta-like scalars returns ``TimedeltaIndex`` instead of raising ``TypeError`` (:issue:`19333`)


Indexing
Expand Down
23 changes: 20 additions & 3 deletions pandas/core/indexes/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import numpy as np
from pandas._libs import (lib, index as libindex, tslib as libts,
algos as libalgos, join as libjoin,
Timestamp)
Timestamp, Timedelta)
from pandas._libs.lib import is_datetime_array

from pandas.compat import range, u, set_function_name
Expand All @@ -16,7 +16,7 @@
from pandas.core.dtypes.generic import (
ABCSeries, ABCDataFrame,
ABCMultiIndex,
ABCPeriodIndex,
ABCPeriodIndex, ABCTimedeltaIndex,
ABCDateOffset)
from pandas.core.dtypes.missing import isna, array_equivalent
from pandas.core.dtypes.common import (
Expand Down Expand Up @@ -3918,7 +3918,21 @@ def dropna(self, how='any'):
return self._shallow_copy()

def _evaluate_with_timedelta_like(self, other, op, opstr, reversed=False):
raise TypeError("can only perform ops with timedelta like values")
# Timedelta knows how to operate with np.array, so dispatch to that
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be left alone, and rather define this in TimedeltaIndex?

Copy link
Member Author

Choose a reason for hiding this comment

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

This for for numeric-dtyped indexes. e.g. pd.Index(range(3)) * pd.Timedelta(days=1) goes through this path.

Copy link
Contributor

Choose a reason for hiding this comment

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

why doesn't this just raise NotImplementedError? which then Timedelta would handle? or is that too recursive a path?

Copy link
Member Author

@jbrockmendel jbrockmendel Jan 21, 2018

Choose a reason for hiding this comment

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

For one thing b/c there’s pytimedelta and timedelta64 that need to be handled.

# operation and then wrap the results
other = Timedelta(other)
values = self.values
if reversed:
values, other = other, values

with np.errstate(all='ignore'):
result = op(values, other)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add some comments (or expand the top ones), this code does a lot


attrs = self._get_attributes_dict()
attrs = self._maybe_update_attributes(attrs)
if op == divmod:
return Index(result[0], **attrs), Index(result[1], **attrs)
return Index(result, **attrs)

def _evaluate_with_datetime_like(self, other, op, opstr):
raise TypeError("can only perform ops with datetime like values")
Expand Down Expand Up @@ -4061,6 +4075,9 @@ def _make_evaluate_binop(op, opstr, reversed=False, constructor=Index):
def _evaluate_numeric_binop(self, other):
if isinstance(other, (ABCSeries, ABCDataFrame)):
return NotImplemented
elif isinstance(other, ABCTimedeltaIndex):
# Defer to subclass implementation
return NotImplemented

other = self._validate_for_numeric_binop(other, op, opstr)

Expand Down
12 changes: 11 additions & 1 deletion pandas/core/indexes/range.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from sys import getsizeof
import operator
from datetime import timedelta

import numpy as np
from pandas._libs import index as libindex
Expand All @@ -8,7 +9,7 @@
is_integer,
is_scalar,
is_int64_dtype)
from pandas.core.dtypes.generic import ABCSeries
from pandas.core.dtypes.generic import ABCSeries, ABCTimedeltaIndex

from pandas import compat
from pandas.compat import lrange, range, get_range_parameters
Expand Down Expand Up @@ -587,6 +588,15 @@ def _make_evaluate_binop(op, opstr, reversed=False, step=False):
def _evaluate_numeric_binop(self, other):
if isinstance(other, ABCSeries):
return NotImplemented
elif isinstance(other, ABCTimedeltaIndex):
# Defer to TimedeltaIndex implementation
return NotImplemented
elif isinstance(other, (timedelta, np.timedelta64)):
# GH#19333 is_integer evaluated True on timedelta64,
# so we need to catch these explicitly
if reversed:
return op(other, self._int64index)
return op(self._int64index, other)

other = self._validate_for_numeric_binop(other, op, opstr)
attrs = self._get_attributes_dict()
Expand Down
38 changes: 37 additions & 1 deletion pandas/tests/indexes/test_numeric.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import pandas.util.testing as tm

import pandas as pd
from pandas._libs.tslib import Timestamp
from pandas._libs.tslib import Timestamp, Timedelta

from pandas.tests.indexes.common import Base

Expand All @@ -26,6 +26,42 @@ def full_like(array, value):
return ret


class TestIndexArithmeticWithTimedeltaScalar(object):

@pytest.mark.parametrize('index', [
Int64Index(range(1, 11)),
UInt64Index(range(1, 11)),
Float64Index(range(1, 11)),
RangeIndex(1, 11)])
@pytest.mark.parametrize('scalar_td', [Timedelta(days=1),
Timedelta(days=1).to_timedelta64(),
Timedelta(days=1).to_pytimedelta()])
def test_index_mul_timedelta(self, scalar_td, index):
# GH#19333
expected = pd.timedelta_range('1 days', '10 days')

result = index * scalar_td
tm.assert_index_equal(result, expected)
commute = scalar_td * index
tm.assert_index_equal(commute, expected)

@pytest.mark.parametrize('index', [Int64Index(range(1, 3)),
UInt64Index(range(1, 3)),
Float64Index(range(1, 3)),
RangeIndex(1, 3)])
@pytest.mark.parametrize('scalar_td', [Timedelta(days=1),
Timedelta(days=1).to_timedelta64(),
Timedelta(days=1).to_pytimedelta()])
def test_index_rdiv_timedelta(self, scalar_td, index):
expected = pd.TimedeltaIndex(['1 Day', '12 Hours'])

result = scalar_td / index
tm.assert_index_equal(result, expected)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you also test that the commute raises here


with pytest.raises(TypeError):
index / scalar_td


class Numeric(Base):

def test_numeric_compat(self):
Expand Down
16 changes: 15 additions & 1 deletion pandas/tests/indexes/timedeltas/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def test_dti_mul_dti_raises(self):

def test_dti_mul_too_short_raises(self):
idx = self._holder(np.arange(5, dtype='int64'))
with pytest.raises(ValueError):
Copy link
Contributor

Choose a reason for hiding this comment

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

why did this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the problem is now caught in TimedeltaIndex when checking what it is operating against, whereas before the problem was caught in numpy when trying to operate on differently-sized arrays.

with pytest.raises(TypeError):
idx * self._holder(np.arange(3))
with pytest.raises(ValueError):
idx * np.array([1, 2])
Expand Down Expand Up @@ -527,6 +527,20 @@ def test_tdi_div_tdlike_scalar_with_nat(self, delta):
result = rng / delta
tm.assert_index_equal(result, expected)

@pytest.mark.parametrize('other', [np.arange(1, 11),
pd.Int64Index(range(1, 11)),
pd.UInt64Index(range(1, 11)),
pd.Float64Index(range(1, 11)),
pd.RangeIndex(1, 11)])
def test_tdi_rmul_arraylike(self, other):
tdi = TimedeltaIndex(['1 Day'] * 10)
expected = timedelta_range('1 days', '10 days')

result = other * tdi
tm.assert_index_equal(result, expected)
commute = tdi * other
tm.assert_index_equal(commute, expected)

def test_subtraction_ops(self):
# with datetimes/timedelta and tdi/dti
tdi = TimedeltaIndex(['1 days', pd.NaT, '2 days'], name='foo')
Expand Down