Skip to content

BUG: Timestamp+int should raise NullFrequencyError, not ValueError #28268

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

Merged
merged 13 commits into from
Sep 8, 2019
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.0.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Datetimelike
- Bug in ``HDFStore.__getitem__`` incorrectly reading tz attribute created in Python 2 (:issue:`26443`)
- Bug in :meth:`pandas.core.groupby.SeriesGroupBy.nunique` where ``NaT`` values were interfering with the count of unique values (:issue:`27951`)
- Bug in :class:`Timestamp` subtraction when subtracting a :class:`Timestamp` from a ``np.datetime64`` object incorrectly raising ``TypeError`` (:issue:`28286`)
- Addition and subtraction of integer or integer-dtype arrays with :class:`Timestamp` will now raise ``NullFrequencyError`` instead of ``ValueError`` (:issue:`28268`)
-


Expand Down
3 changes: 3 additions & 0 deletions pandas/_libs/tslibs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@
from .timedeltas import Timedelta, delta_to_nanoseconds, ints_to_pytimedelta
from .timestamps import Timestamp
from .tzconversion import tz_convert_single

# import fails if we do this before np_datetime
from .c_timestamp import NullFrequencyError # isort:skip
34 changes: 20 additions & 14 deletions pandas/_libs/tslibs/c_timestamp.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ from pandas._libs.tslibs.timezones import UTC
from pandas._libs.tslibs.tzconversion cimport tz_convert_single


class NullFrequencyError(ValueError):
"""
Error raised when a null `freq` attribute is used in an operation
that needs a non-null frequency, particularly `DatetimeIndex.shift`,
`TimedeltaIndex.shift`, `PeriodIndex.shift`.
"""
pass


def maybe_integer_op_deprecated(obj):
# GH#22535 add/sub of integers and int-arrays is deprecated
if obj.freq is not None:
Expand Down Expand Up @@ -227,8 +236,8 @@ cdef class _Timestamp(datetime):
# to be compat with Period
return NaT
elif self.freq is None:
raise ValueError("Cannot add integral value to Timestamp "
"without freq.")
raise NullFrequencyError(
"Cannot add integral value to Timestamp without freq.")
return self.__class__((self.freq * other).apply(self),
freq=self.freq)

Expand All @@ -246,17 +255,15 @@ cdef class _Timestamp(datetime):

result = self.__class__(self.value + nanos,
tz=self.tzinfo, freq=self.freq)
if getattr(other, 'normalize', False):
# DateOffset
result = result.normalize()
return result

elif is_array(other):
if other.dtype.kind in ['i', 'u']:
maybe_integer_op_deprecated(self)
if self.freq is None:
raise ValueError("Cannot add integer-dtype array "
"to Timestamp without freq.")
raise NullFrequencyError(
"Cannot add integer-dtype array "
"to Timestamp without freq.")
return self.freq * other + self

# index/series like
Expand All @@ -270,6 +277,7 @@ cdef class _Timestamp(datetime):
return result

def __sub__(self, other):

if (is_timedelta64_object(other) or is_integer_object(other) or
PyDelta_Check(other) or hasattr(other, 'delta')):
# `delta` attribute is for offsets.Tick or offsets.Week obj
Expand All @@ -280,15 +288,16 @@ cdef class _Timestamp(datetime):
if other.dtype.kind in ['i', 'u']:
maybe_integer_op_deprecated(self)
if self.freq is None:
raise ValueError("Cannot subtract integer-dtype array "
"from Timestamp without freq.")
raise NullFrequencyError(
"Cannot subtract integer-dtype array "
"from Timestamp without freq.")
return self - self.freq * other

typ = getattr(other, '_typ', None)
if typ is not None:
return NotImplemented

elif other is NaT:
if other is NaT:
return NaT

# coerce if necessary if we are a Timestamp-like
Expand All @@ -311,15 +320,12 @@ cdef class _Timestamp(datetime):
return Timedelta(self.value - other.value)
except (OverflowError, OutOfBoundsDatetime):
pass

elif is_datetime64_object(self):
# GH#28286 cython semantics for __rsub__, `other` is actually
# the Timestamp
return type(other)(self) - other

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

cdef int64_t _maybe_convert_value_to_local(self):
"""Convert UTC i8 value to local i8 value if tz exists"""
Expand Down
10 changes: 1 addition & 9 deletions pandas/errors/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
Expose public exceptions & warnings
"""

from pandas._libs.tslibs import OutOfBoundsDatetime
from pandas._libs.tslibs import NullFrequencyError, OutOfBoundsDatetime


class PerformanceWarning(Warning):
Expand Down Expand Up @@ -157,14 +157,6 @@ class MergeError(ValueError):
"""


class NullFrequencyError(ValueError):
"""
Error raised when a null `freq` attribute is used in an operation
that needs a non-null frequency, particularly `DatetimeIndex.shift`,
`TimedeltaIndex.shift`, `PeriodIndex.shift`.
"""


class AccessorRegistrationWarning(Warning):
"""Warning for attribute conflicts in accessor registration."""

Expand Down
5 changes: 1 addition & 4 deletions pandas/tests/arithmetic/test_timedelta64.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,10 +241,7 @@ def test_subtraction_ops(self):
with pytest.raises(TypeError, match=msg):
tdi - dti

msg = (
r"descriptor '__sub__' requires a 'datetime\.datetime' object"
" but received a 'Timedelta'"
)
msg = r"unsupported operand type\(s\) for -"
with pytest.raises(TypeError, match=msg):
td - dt

Expand Down
8 changes: 5 additions & 3 deletions pandas/tests/scalar/timestamp/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import numpy as np
import pytest

from pandas.errors import NullFrequencyError

from pandas import Timedelta, Timestamp
import pandas.util.testing as tm

Expand Down Expand Up @@ -177,12 +179,12 @@ def test_timestamp_add_timedelta64_unit(self, other, expected_difference):
],
)
def test_add_int_no_freq_raises(self, ts, other):
with pytest.raises(ValueError, match="without freq"):
with pytest.raises(NullFrequencyError, match="without freq"):
ts + other
with pytest.raises(ValueError, match="without freq"):
with pytest.raises(NullFrequencyError, match="without freq"):
other + ts

with pytest.raises(ValueError, match="without freq"):
with pytest.raises(NullFrequencyError, match="without freq"):
ts - other
with pytest.raises(TypeError):
other - ts
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/tslibs/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def test_namespace():
"NaTType",
"iNaT",
"is_null_datetimelike",
"NullFrequencyError",
"OutOfBoundsDatetime",
"Period",
"IncompatibleFrequency",
Expand Down