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 all 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
115 changes: 72 additions & 43 deletions pandas/core/ops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,54 +386,19 @@ 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

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 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))):

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)):
def _validate_datetime(self, lvalues, rvalues, name):
# assumes self.is_datetime_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 and
(self.is_timedelta_lhs or self.is_offset_lhs)):
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))

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

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"
Expand All @@ -444,18 +409,82 @@ def _validate(self, lvalues, rvalues, name):
raise ValueError("Incompatible tz's on datetime subtraction "
"ops")

elif ((self.is_timedelta_lhs or self.is_offset_lhs) and
self.is_datetime_rhs):
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
self._check_timedelta_with_numeric(name)
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))
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')

def _validate_offset(self, name):
# assumes self.is_offset_lhs

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):
self._check_timedelta_with_numeric(name)
else:
raise TypeError('cannot operate on a series without a rhs '
'of a series/ndarray of type datetime64[ns] '
'or a timedelta')

def _check_timedelta_with_numeric(self, name):
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))

def _convert_to_array(self, values, name=None, other=None):
"""converts values to ndarray"""
from pandas.core.tools.timedeltas import to_timedelta
Expand Down
53 changes: 43 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,51 @@ def test_timedelta64_ops_nat(self):
assert_series_equal(timedelta_series / nan,
nat_series_dtype_timedelta)

@pytest.mark.parametrize('scalar_td', [timedelta(minutes=5, seconds=4),
Timedelta(minutes=5, seconds=4),
Timedelta('5m4s').to_timedelta64()])
def test_operators_timedelta64_with_timedelta(self, scalar_td):
# smoke tests
td1 = Series([timedelta(minutes=5, seconds=3)] * 3)
td1.iloc[2] = np.nan

td1 + scalar_td
scalar_td + td1
td1 - scalar_td
scalar_td - td1
td1 / scalar_td
scalar_td / td1

@pytest.mark.parametrize('scalar_td', [
timedelta(minutes=5, seconds=4),
pytest.param(Timedelta('5m4s'),
marks=pytest.mark.xfail(reason="Timedelta.__floordiv__ "
"bug GH#18846")),
Timedelta('5m4s').to_timedelta64()])
def test_operators_timedelta64_with_timedelta_invalid(self, scalar_td):
td1 = Series([timedelta(minutes=5, seconds=3)] * 3)
td1.iloc[2] = np.nan

# 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 * scalar_td
with tm.assert_raises_regex(TypeError, pattern):
scalar_td * td1
with tm.assert_raises_regex(TypeError, pattern):
td1 // scalar_td
with tm.assert_raises_regex(TypeError, pattern):
scalar_td // td1
with tm.assert_raises_regex(TypeError, pattern):
scalar_td ** td1
with tm.assert_raises_regex(TypeError, pattern):
td1 ** scalar_td


class TestDatetimeSeriesArithmetic(object):

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

Expand All @@ -976,16 +1019,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