Skip to content

BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865) #8981

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 4, 2014
Merged

BUG: Bug in Timestamp-Timestamp not returning a Timedelta type (GH8865) #8981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 18 additions & 0 deletions doc/source/whatsnew/v0.15.2.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,24 @@ Experimental

Bug Fixes
~~~~~~~~~

- Bug in Timestamp-Timestamp not returning a Timedelta type and datelike-datelike ops with timezones (:issue:`8865`)
- Made consistent a timezone mismatch exception (either tz operated with None or incompatible timezone), will now return ``TypeError`` rather than ``ValueError`` (a couple of edge cases only), (:issue:`8865`)
- Report a ``TypeError`` when invalid/no paramaters are passed in a groupby (:issue:`8015`)
- Bug in packaging pandas with ``py2app/cx_Freeze`` (:issue:`8602`, :issue:`8831`)
- Bug in ``groupby`` signatures that didn't include \*args or \*\*kwargs (:issue:`8733`).
- ``io.data.Options`` now raises ``RemoteDataError`` when no expiry dates are available from Yahoo and when it receives no data from Yahoo (:issue:`8761`), (:issue:`8783`).
- Unclear error message in csv parsing when passing dtype and names and the parsed data is a different data type (:issue:`8833`)
- Bug in slicing a multi-index with an empty list and at least one boolean indexer (:issue:`8781`)
- ``io.data.Options`` now raises ``RemoteDataError`` when no expiry dates are available from Yahoo (:issue:`8761`).
- ``Timedelta`` kwargs may now be numpy ints and floats (:issue:`8757`).
- Fixed several outstanding bugs for ``Timedelta`` arithmetic and comparisons (:issue:`8813`, :issue:`5963`, :issue:`5436`).
- ``sql_schema`` now generates dialect appropriate ``CREATE TABLE`` statements (:issue:`8697`)
- ``slice`` string method now takes step into account (:issue:`8754`)
- Bug in ``BlockManager`` where setting values with different type would break block integrity (:issue:`8850`)
- Bug in ``DatetimeIndex`` when using ``time`` object as key (:issue:`8667`)
- Bug in ``merge`` where ``how='left'`` and ``sort=False`` would not preserve left frame order (:issue:`7331`)

- Fix negative step support for label-based slices (:issue:`8753`)

Old behavior:
Expand Down
4 changes: 2 additions & 2 deletions pandas/tests/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,8 +285,8 @@ def test_ops(self):
expected = pd.Period(ordinal=getattr(o.values, op)(), freq=o.freq)
try:
self.assertEqual(result, expected)
except ValueError:
# comparing tz-aware series with np.array results in ValueError
except TypeError:
# comparing tz-aware series with np.array results in TypeError
expected = expected.astype('M8[ns]').astype('int64')
self.assertEqual(result.value, expected)

Expand Down
2 changes: 1 addition & 1 deletion pandas/tseries/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ def __sub__(self, other):
cls.__sub__ = __sub__

def __rsub__(self, other):
return -self + other
return -(self - other)
cls.__rsub__ = __rsub__

cls.__iadd__ = __add__
Expand Down
7 changes: 6 additions & 1 deletion pandas/tseries/index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ def _generate(cls, start, end, periods, name, offset,
try:
inferred_tz = tools._infer_tzinfo(start, end)
except:
raise ValueError('Start and end cannot both be tz-aware with '
raise TypeError('Start and end cannot both be tz-aware with '
'different timezones')

inferred_tz = tslib.maybe_get_tz(inferred_tz)
Expand Down Expand Up @@ -645,6 +645,11 @@ def _sub_datelike(self, other):

from pandas import TimedeltaIndex
other = Timestamp(other)

# require tz compat
if tslib.get_timezone(self.tz) != tslib.get_timezone(other.tzinfo):
raise TypeError("Timestamp subtraction must have the same timezones or no timezones")

i8 = self.asi8
result = i8 - other.value
result = self._maybe_mask_results(result,fill_value=tslib.iNaT)
Expand Down
68 changes: 68 additions & 0 deletions pandas/tseries/tests/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,74 @@ def test_subtraction_ops(self):
expected = DatetimeIndex(['20121231',pd.NaT,'20121230'])
tm.assert_index_equal(result,expected)

def test_subtraction_ops_with_tz(self):

# check that dt/dti subtraction ops with tz are validated
dti = date_range('20130101',periods=3)
ts = Timestamp('20130101')
dt = ts.to_datetime()
dti_tz = date_range('20130101',periods=3).tz_localize('US/Eastern')
ts_tz = Timestamp('20130101').tz_localize('US/Eastern')
ts_tz2 = Timestamp('20130101').tz_localize('CET')
dt_tz = ts_tz.to_datetime()
td = Timedelta('1 days')

def _check(result, expected):
self.assertEqual(result,expected)
self.assertIsInstance(result, Timedelta)

# scalars
result = ts - ts
expected = Timedelta('0 days')
_check(result, expected)

result = dt_tz - ts_tz
expected = Timedelta('0 days')
_check(result, expected)

result = ts_tz - dt_tz
expected = Timedelta('0 days')
_check(result, expected)

# tz mismatches
self.assertRaises(TypeError, lambda : dt_tz - ts)
self.assertRaises(TypeError, lambda : dt_tz - dt)
self.assertRaises(TypeError, lambda : dt_tz - ts_tz2)
self.assertRaises(TypeError, lambda : dt - dt_tz)
self.assertRaises(TypeError, lambda : ts - dt_tz)
self.assertRaises(TypeError, lambda : ts_tz2 - ts)
self.assertRaises(TypeError, lambda : ts_tz2 - dt)
self.assertRaises(TypeError, lambda : ts_tz - ts_tz2)

# with dti
self.assertRaises(TypeError, lambda : dti - ts_tz)
self.assertRaises(TypeError, lambda : dti_tz - ts)
self.assertRaises(TypeError, lambda : dti_tz - ts_tz2)

result = dti_tz-dt_tz
expected = TimedeltaIndex(['0 days','1 days','2 days'])
tm.assert_index_equal(result,expected)

result = dt_tz-dti_tz
expected = TimedeltaIndex(['0 days','-1 days','-2 days'])
tm.assert_index_equal(result,expected)

result = dti_tz-ts_tz
expected = TimedeltaIndex(['0 days','1 days','2 days'])
tm.assert_index_equal(result,expected)

result = ts_tz-dti_tz
expected = TimedeltaIndex(['0 days','-1 days','-2 days'])
tm.assert_index_equal(result,expected)

result = td - td
expected = Timedelta('0 days')
_check(result, expected)

result = dti_tz - td
expected = DatetimeIndex(['20121231','20130101','20130102'],tz='US/Eastern')
tm.assert_index_equal(result,expected)

def test_dti_tdi_numeric_ops(self):

# These are normally union/diff set-like ops
Expand Down
12 changes: 6 additions & 6 deletions pandas/tseries/tests/test_tslib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pandas import tslib
import datetime

from pandas.core.api import Timestamp, Series
from pandas.core.api import Timestamp, Series, Timedelta
from pandas.tslib import period_asfreq, period_ordinal, get_timezone
from pandas.tseries.index import date_range
from pandas.tseries.frequencies import get_freq
Expand Down Expand Up @@ -232,13 +232,13 @@ def test_tz(self):
conv = local.tz_convert('US/Eastern')
self.assertEqual(conv.nanosecond, 5)
self.assertEqual(conv.hour, 19)

def test_tz_localize_ambiguous(self):

ts = Timestamp('2014-11-02 01:00')
ts_dst = ts.tz_localize('US/Eastern', ambiguous=True)
ts_no_dst = ts.tz_localize('US/Eastern', ambiguous=False)

rng = date_range('2014-11-02', periods=3, freq='H', tz='US/Eastern')
self.assertEqual(rng[1], ts_dst)
self.assertEqual(rng[2], ts_no_dst)
Expand Down Expand Up @@ -679,8 +679,8 @@ def test_addition_subtraction_types(self):
self.assertEqual(type(timestamp_instance - 1), Timestamp)

# Timestamp + datetime not supported, though subtraction is supported and yields timedelta
self.assertEqual(type(timestamp_instance - datetime_instance), datetime.timedelta)

# more tests in tseries/base/tests/test_base.py
self.assertEqual(type(timestamp_instance - datetime_instance), Timedelta)
self.assertEqual(type(timestamp_instance + timedelta_instance), Timestamp)
self.assertEqual(type(timestamp_instance - timedelta_instance), Timestamp)

Expand Down
26 changes: 24 additions & 2 deletions pandas/tslib.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,10 +810,10 @@ cdef class _Timestamp(datetime):
object other) except -1:
if self.tzinfo is None:
if other.tzinfo is not None:
raise ValueError('Cannot compare tz-naive and tz-aware '
raise TypeError('Cannot compare tz-naive and tz-aware '
'timestamps')
elif other.tzinfo is None:
raise ValueError('Cannot compare tz-naive and tz-aware timestamps')
raise TypeError('Cannot compare tz-naive and tz-aware timestamps')

cpdef datetime to_datetime(_Timestamp self):
cdef:
Expand Down Expand Up @@ -863,6 +863,11 @@ cdef class _Timestamp(datetime):

# a Timestamp-DatetimeIndex -> yields a negative TimedeltaIndex
elif getattr(other,'_typ',None) == 'datetimeindex':

# we may be passed reverse ops
if get_timezone(getattr(self,'tzinfo',None)) != get_timezone(other.tz):
raise TypeError("Timestamp subtraction must have the same timezones or no timezones")
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure timezone comparison is a ValueError... it would be good to be consistent (IMO TypeError is the right choice)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Pdb) p dt_tz
datetime.datetime(2013, 1, 1, 0, 0, tzinfo=<DstTzInfo 'US/Eastern' EST-1 day, 19:00:00 STD>)
(Pdb) p dt
datetime.datetime(2013, 1, 1, 0, 0)
(Pdb) p dt_tz - dt
*** TypeError: TypeError("can't subtract offset-naive and offset-aware datetimes",)

datetime-datetime has this as a TypeError (that's why I changed it, originally I had it as a ValueError as well)

Copy link
Member

Choose a reason for hiding this comment

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

OK, so maybe you should also change this for comparisons on line 813 above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixing this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess its bad when I change from ValueError to TypeError and no tests break :)

It was almost all TypeError, so this is the right move.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually was a failed test so that is good :)


return -other.__sub__(self)

# a Timestamp-TimedeltaIndex -> yields a negative TimedeltaIndex
Expand All @@ -871,6 +876,23 @@ cdef class _Timestamp(datetime):

elif other is NaT:
return NaT

# coerce if necessary if we are a Timestamp-like
if isinstance(self, datetime) and (isinstance(other, datetime) or is_datetime64_object(other)):
self = Timestamp(self)
other = Timestamp(other)

# validate tz's
if get_timezone(self.tzinfo) != get_timezone(other.tzinfo):
raise TypeError("Timestamp subtraction must have the same timezones or no timezones")

# scalar Timestamp/datetime - Timestamp/datetime -> yields a Timedelta
try:
return Timedelta(self.value-other.value)
except (OverflowError, OutOfBoundsDatetime):
pass

# scalar Timestamp/datetime - Timedelta -> yields a Timestamp (with same timezone if specified)
return datetime.__sub__(self, other)

cpdef _get_field(self, field):
Expand Down