Skip to content

Refactor _TimeOp._validate to separate datetime vs timedelta vs dateoffset #18832

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 7 commits into from
Dec 23, 2017
Merged

Refactor _TimeOp._validate to separate datetime vs timedelta vs dateoffset #18832

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
49aa384
refactor ops to isolate datetime and timedelta logic
jbrockmendel Dec 18, 2017
1d7903e
revert out of scope
jbrockmendel Dec 19, 2017
2809a41
Revert out of scope
jbrockmendel Dec 19, 2017
d005760
revert out of scope
jbrockmendel Dec 19, 2017
06bf523
make test more explicit
jbrockmendel Dec 19, 2017
cbc04b7
rename duplicate test
jbrockmendel Dec 19, 2017
a3cab61
edits per request; parametrize tests
jbrockmendel Dec 21, 2017
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
111 changes: 71 additions & 40 deletions pandas/core/ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,71 +386,102 @@ def __init__(self, left, right, name, na_op):
self.lvalues, self.rvalues = self._convert_for_datetime(lvalues,
rvalues)

def _validate(self, lvalues, rvalues, name):
# timedelta and integer mul/div
def _validate_datetime(self, lvalues, rvalues, name):
# assumes self.is_datetime_lhs

if ((self.is_timedelta_lhs and
(self.is_integer_rhs or self.is_floating_rhs)) or
(self.is_timedelta_rhs and
(self.is_integer_lhs or self.is_floating_lhs))):
if (self.is_timedelta_rhs or self.is_offset_rhs):
# datetime and timedelta/DateOffset
if name not in ('__add__', '__radd__', '__sub__'):
raise TypeError("can only operate on a datetime with a rhs of "
"a timedelta/DateOffset for addition and "
"subtraction, but the operator [{name}] was "
"passed".format(name=name))

elif self.is_datetime_rhs:
# 2 datetimes
if name not in ('__sub__', '__rsub__'):
raise TypeError("can only operate on a datetimes for"
" subtraction, but the operator [{name}] was"
" passed".format(name=name))

# if tz's must be equal (same or None)
if getattr(lvalues, 'tz', None) != getattr(rvalues, 'tz', None):
raise ValueError("Incompatible tz's on datetime subtraction "
"ops")

else:
raise TypeError('cannot operate on a series without a rhs '
'of a series/ndarray of type datetime64[ns] '
'or a timedelta')

def _validate_timedelta(self, name):
# assumes self.is_timedelta_lhs

if self.is_integer_rhs or self.is_floating_rhs:
# timedelta and integer mul/div
if name not in ('__div__', '__truediv__', '__mul__', '__rmul__'):
raise TypeError("can only operate on a timedelta and an "
"integer or a float for division and "
"multiplication, but the operator [{name}] "
"was passed".format(name=name))

# 2 timedeltas
elif ((self.is_timedelta_lhs and
(self.is_timedelta_rhs or self.is_offset_rhs)) or
(self.is_timedelta_rhs and
(self.is_timedelta_lhs or self.is_offset_lhs))):

elif self.is_timedelta_rhs or self.is_offset_rhs:
# 2 timedeltas
if name not in ('__div__', '__rdiv__', '__truediv__',
'__rtruediv__', '__add__', '__radd__', '__sub__',
'__rsub__'):
raise TypeError("can only operate on a timedeltas for addition"
", subtraction, and division, but the operator"
" [{name}] was passed".format(name=name))

# datetime and timedelta/DateOffset
elif (self.is_datetime_lhs and
(self.is_timedelta_rhs or self.is_offset_rhs)):

if name not in ('__add__', '__radd__', '__sub__'):
raise TypeError("can only operate on a datetime with a rhs of "
"a timedelta/DateOffset for addition and "
"subtraction, but the operator [{name}] was "
"passed".format(name=name))

elif (self.is_datetime_rhs and
(self.is_timedelta_lhs or self.is_offset_lhs)):
elif self.is_datetime_rhs:
if name not in ('__add__', '__radd__', '__rsub__'):
raise TypeError("can only operate on a timedelta/DateOffset "
"with a rhs of a datetime for addition, "
"but the operator [{name}] was passed"
.format(name=name))
else:
raise TypeError('cannot operate on a series without a rhs '
'of a series/ndarray of type datetime64[ns] '
'or a timedelta')

# 2 datetimes
elif self.is_datetime_lhs and self.is_datetime_rhs:

if name not in ('__sub__', '__rsub__'):
raise TypeError("can only operate on a datetimes for"
" subtraction, but the operator [{name}] was"
" passed".format(name=name))
def _validate_offset(self, name):
# assumes self.is_offset_lhs

# if tz's must be equal (same or None)
if getattr(lvalues, 'tz', None) != getattr(rvalues, 'tz', None):
raise ValueError("Incompatible tz's on datetime subtraction "
"ops")

elif ((self.is_timedelta_lhs or self.is_offset_lhs) and
self.is_datetime_rhs):
if self.is_timedelta_rhs:
# 2 timedeltas
if name not in ('__div__', '__rdiv__', '__truediv__',
'__rtruediv__', '__add__', '__radd__', '__sub__',
'__rsub__'):
raise TypeError("can only operate on a timedeltas for addition"
", subtraction, and division, but the operator"
" [{name}] was passed".format(name=name))

elif self.is_datetime_rhs:
if name not in ('__add__', '__radd__'):
raise TypeError("can only operate on a timedelta/DateOffset "
"and a datetime for addition, but the operator"
" [{name}] was passed".format(name=name))

else:
raise TypeError('cannot operate on a series without a rhs '
'of a series/ndarray of type datetime64[ns] '
'or a timedelta')

def _validate(self, lvalues, rvalues, name):
if self.is_datetime_lhs:
return self._validate_datetime(lvalues, rvalues, name)
elif self.is_timedelta_lhs:
return self._validate_timedelta(name)
elif self.is_offset_lhs:
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is ok for now. Maybe should make 3 sub-classes here (or I think ultimately refactoring to have all this logic in the index methods as it already duplicates some of this).

return self._validate_offset(name)

if ((self.is_integer_lhs or self.is_floating_lhs) and
self.is_timedelta_rhs):

if name not in ('__div__', '__truediv__', '__mul__', '__rmul__'):
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe factor this to a function to avoid duplication with above,

assert_can_operate() or somesuch

raise TypeError("can only operate on a timedelta and an "
"integer or a float for division and "
"multiplication, but the operator [{name}] "
"was passed".format(name=name))
else:
raise TypeError('cannot operate on a series without a rhs '
'of a series/ndarray of type datetime64[ns] '
Expand Down
46 changes: 36 additions & 10 deletions pandas/tests/series/test_operators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -960,8 +960,44 @@ def test_timedelta64_ops_nat(self):
assert_series_equal(timedelta_series / nan,
nat_series_dtype_timedelta)

def test_operators_timedelta64_with_timedelta(self):
# smoke tests
td1 = Series([timedelta(minutes=5, seconds=3)] * 3)
Copy link
Contributor

Choose a reason for hiding this comment

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

we certainly have some of these, can you remove the existing ones if you are consolidating.

these also seem oddly specific, would parametrize on Timedelta, TDI as well here

Copy link
Member Author

Choose a reason for hiding this comment

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

we certainly have some of these, can you remove the existing ones if you are consolidating.

Yes, these are already moved out of the giant .test_operators_datetimelike

these also seem oddly specific, would parametrize on Timedelta, TDI as well here

Not sure about TDI, but definitely can parametrize over scalar timedelta variants.

td1.iloc[2] = np.nan
td2 = timedelta(minutes=5, seconds=4)

td1 + td2
td2 + td1
td1 - td2
td2 - td1
td1 / td2
td2 / td1

def test_operators_timedelta64_with_timedelta_invalid(self):
td1 = Series([timedelta(minutes=5, seconds=3)] * 3)
td1.iloc[2] = np.nan
Copy link
Contributor

Choose a reason for hiding this comment

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

same parametization

td2 = timedelta(minutes=5, seconds=4)

# check that we are getting a TypeError
# with 'operate' (from core/ops.py) for the ops that are not
# defined
pattern = 'operate|unsupported|cannot'
with tm.assert_raises_regex(TypeError, pattern):
td1 * td2
with tm.assert_raises_regex(TypeError, pattern):
td2 * td1
with tm.assert_raises_regex(TypeError, pattern):
td1 // td2
with tm.assert_raises_regex(TypeError, pattern):
td2 // td1
with tm.assert_raises_regex(TypeError, pattern):
td2 ** td1
with tm.assert_raises_regex(TypeError, pattern):
td1 ** td2


class TestDatetimeSeriesArithmetic(object):

def test_operators_datetimelike(self):
def run_ops(ops, get_ser, test_ser):

Expand All @@ -976,16 +1012,6 @@ def run_ops(ops, get_ser, test_ser):
# ## timedelta64 ###
Copy link
Contributor

Choose a reason for hiding this comment

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

is this initialization code then used below?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes.

td1 = Series([timedelta(minutes=5, seconds=3)] * 3)
td1.iloc[2] = np.nan
td2 = timedelta(minutes=5, seconds=4)
ops = ['__mul__', '__floordiv__', '__pow__', '__rmul__',
'__rfloordiv__', '__rpow__']
run_ops(ops, td1, td2)
td1 + td2
td2 + td1
td1 - td2
td2 - td1
td1 / td2
td2 / td1

# ## datetime64 ###
dt1 = Series([Timestamp('20111230'), Timestamp('20120101'),
Expand Down