Skip to content

Remove unused/unreachable code from ops #18964

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

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
133 changes: 35 additions & 98 deletions pandas/core/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ def add_methods(cls, new_methods, force, select, exclude):
raise TypeError("May only pass either select or exclude")

if select:
# TODO: This is not hit in coverage report. Is it still needed?
select = set(select)
methods = {}
for key, method in new_methods.items():
Expand All @@ -164,6 +165,7 @@ def add_methods(cls, new_methods, force, select, exclude):
new_methods = methods

if exclude:
# TODO: This is not hit in coverage report. Is it still needed?
for k in exclude:
new_methods.pop(k, None)

Expand Down Expand Up @@ -354,24 +356,26 @@ class _TimeOp(_Op):
"""
Wrapper around Series datetime/time/timedelta arithmetic operations.
Generally, you should use classmethod ``_Op.get_op`` as an entry point.

This is only reached in cases where either self.is_datetime_lhs
or self.is_timedelta_lhs.
"""
fill_value = iNaT

def __init__(self, left, right, name, na_op):
super(_TimeOp, self).__init__(left, right, name, na_op)

lvalues = self._convert_to_array(left, name=name)
rvalues = self._convert_to_array(right, name=name, other=lvalues)
lvalues = self._convert_to_array(left)
rvalues = self._convert_to_array(right, other=lvalues)

# left
self.is_offset_lhs = is_offsetlike(left)
self.is_timedelta_lhs = is_timedelta64_dtype(lvalues)
self.is_datetime64_lhs = is_datetime64_dtype(lvalues)
self.is_datetime64tz_lhs = is_datetime64tz_dtype(lvalues)
self.is_datetime_lhs = (self.is_datetime64_lhs or
self.is_datetime64tz_lhs)
self.is_integer_lhs = left.dtype.kind in ['i', 'u']
self.is_floating_lhs = left.dtype.kind == 'f'
assert left.dtype.kind not in ['i', 'u', 'f'], left
Copy link
Contributor

Choose a reason for hiding this comment

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

assert one of the is_* e.g.
asert self.is_timedelta_lhs or self is_datetime_lhs

assert self.is_timedelta_lhs or self.is_datetime_lhs

# right
self.is_offset_rhs = is_offsetlike(right)
Expand Down Expand Up @@ -440,44 +444,12 @@ def _validate_timedelta(self, name):
'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:
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')
# The only other option is self.is_timedelta_lhs
return self._validate_timedelta(name)

def _check_timedelta_with_numeric(self, name):
if name not in ('__div__', '__truediv__', '__mul__', '__rmul__'):
Expand All @@ -486,9 +458,10 @@ def _check_timedelta_with_numeric(self, name):
"multiplication, but the operator [{name}] "
"was passed".format(name=name))

def _convert_to_array(self, values, name=None, other=None):
def _convert_to_array(self, values, other=None):
"""converts values to ndarray"""
from pandas.core.tools.timedeltas import to_timedelta
name = self.name

ovalues = values
supplied_dtype = None
Expand Down Expand Up @@ -516,17 +489,13 @@ def _convert_to_array(self, values, name=None, other=None):
# a datelike
elif isinstance(values, pd.DatetimeIndex):
values = values.to_series()
# datetime with tz
elif (isinstance(ovalues, datetime.datetime) and
hasattr(ovalues, 'tzinfo')):
elif isinstance(ovalues, (datetime.datetime, np.datetime64)):
# original input was scalar datetimelike
values = pd.DatetimeIndex(values)
# datetime array with tz
elif is_datetimetz(values):
if isinstance(values, ABCSeries):
values = values._values
elif not (isinstance(values, (np.ndarray, ABCSeries)) and
is_datetime64_dtype(values)):
values = libts.array_to_datetime(values)
elif (is_datetime64_dtype(values) and
not is_datetime64_ns_dtype(values)):
# GH#7996 e.g. np.datetime64('2013-01-01') is datetime64[D]
Expand Down Expand Up @@ -569,10 +538,8 @@ def _convert_for_datetime(self, lvalues, rvalues):

# datetime subtraction means timedelta
if self.is_datetime_lhs and self.is_datetime_rhs:
if self.name in ('__sub__', '__rsub__'):
self.dtype = 'timedelta64[ns]'
else:
self.dtype = 'datetime64[ns]'
assert self.name in ('__sub__', '__rsub__')
self.dtype = 'timedelta64[ns]'
elif self.is_datetime64tz_lhs:
self.dtype = lvalues.dtype
elif self.is_datetime64tz_rhs:
Expand All @@ -595,9 +562,7 @@ def _offset(lvalues, rvalues):
self.na_op = lambda x, y: getattr(x, self.name)(y)
return lvalues, rvalues

if self.is_offset_lhs:
lvalues, rvalues = _offset(lvalues, rvalues)
elif self.is_offset_rhs:
if self.is_offset_rhs:
rvalues, lvalues = _offset(rvalues, lvalues)
else:

Expand All @@ -616,8 +581,6 @@ def _offset(lvalues, rvalues):
self.dtype = 'timedelta64[ns]'

# convert Tick DateOffset to underlying delta
if self.is_offset_lhs:
lvalues = to_timedelta(lvalues, box=False)
if self.is_offset_rhs:
rvalues = to_timedelta(rvalues, box=False)

Expand All @@ -628,7 +591,7 @@ def _offset(lvalues, rvalues):
# time delta division -> unit less
# integer gets converted to timedelta in np < 1.6
if ((self.is_timedelta_lhs and self.is_timedelta_rhs) and
not self.is_integer_rhs and not self.is_integer_lhs and
not self.is_integer_rhs and
self.name in ('__div__', '__rdiv__',
'__truediv__', '__rtruediv__',
'__floordiv__', '__rfloordiv__')):
Expand Down Expand Up @@ -722,23 +685,20 @@ def na_op(x, y):
result = missing.fill_zeros(result, x, y, name, fill_zeros)
return result

def safe_na_op(lvalues, rvalues):
def safe_na_op(lvalues, rvalues, na_op):
# We pass na_op explicitly here for namespace/closure reasons;
# the alternative is to make it an argument to `wrapper`, which
# would mess with the signatures of Series methods.
try:
with np.errstate(all='ignore'):
return na_op(lvalues, rvalues)
except Exception:
if isinstance(rvalues, ABCSeries):
if is_object_dtype(rvalues):
# if dtype is object, try elementwise op
return libalgos.arrmap_object(rvalues,
lambda x: op(lvalues, x))
else:
if is_object_dtype(lvalues):
return libalgos.arrmap_object(lvalues,
lambda x: op(x, rvalues))
if is_object_dtype(lvalues):
return libalgos.arrmap_object(lvalues,
lambda x: op(x, rvalues))
raise

def wrapper(left, right, name=name, na_op=na_op):
def wrapper(left, right):

if isinstance(right, ABCDataFrame):
return NotImplemented
Expand All @@ -747,35 +707,29 @@ def wrapper(left, right, name=name, na_op=na_op):

converted = _Op.get_op(left, right, name, na_op)

left, right = converted.left, converted.right
lvalues, rvalues = converted.lvalues, converted.rvalues
dtype = converted.dtype
wrap_results = converted.wrap_results
na_op = converted.na_op

if isinstance(rvalues, ABCSeries):
name = _maybe_match_name(left, rvalues)
res_name = _maybe_match_name(left, rvalues)
lvalues = getattr(lvalues, 'values', lvalues)
rvalues = getattr(rvalues, 'values', rvalues)
# _Op aligns left and right
else:
if isinstance(rvalues, pd.Index):
name = _maybe_match_name(left, rvalues)
res_name = _maybe_match_name(left, rvalues)
else:
name = left.name
res_name = left.name
if (hasattr(lvalues, 'values') and
not isinstance(lvalues, pd.DatetimeIndex)):
lvalues = lvalues.values

result = wrap_results(safe_na_op(lvalues, rvalues))
return construct_result(
left,
result,
index=left.index,
name=name,
dtype=dtype,
)
result = wrap_results(safe_na_op(lvalues, rvalues, converted.na_op))
return construct_result(left, result,
index=left.index, name=res_name, dtype=dtype)

wrapper.__name__ = name
return wrapper


Expand Down Expand Up @@ -858,7 +812,7 @@ def na_op(x, y):
def wrapper(self, other, axis=None):
# Validate the axis parameter
if axis is not None:
self._get_axis_number(axis)
axis = self._get_axis_number(axis)

if isinstance(other, ABCSeries):
name = _maybe_match_name(self, other)
Expand Down Expand Up @@ -1381,23 +1335,6 @@ def f(self, other):

def _arith_method_PANEL(op, name, str_rep=None, fill_zeros=None,
default_axis=None, **eval_kwargs):
# copied from Series na_op above, but without unnecessary branch for
# non-scalar
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, not super concerned about Panel things generally.

def na_op(x, y):
import pandas.core.computation.expressions as expressions

try:
result = expressions.evaluate(op, str_rep, x, y, **eval_kwargs)
except TypeError:

# TODO: might need to find_common_type here?
result = np.empty(len(x), dtype=x.dtype)
mask = notna(x)
result[mask] = op(x[mask], y)
result, changed = maybe_upcast_putmask(result, ~mask, np.nan)

result = missing.fill_zeros(result, x, y, name, fill_zeros)
return result

# work only for scalars
def f(self, other):
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/series/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -752,3 +752,17 @@ def test_dt_accessor_api_for_categorical(self):
AttributeError, "Can only use .dt accessor with datetimelike"):
invalid.dt
assert not hasattr(invalid, 'str')


@pytest.mark.parametrize('opname', [
'__add__', '__radd__',
'__sub__', '__rsub__',
'__mul__', '__rmul__',
# '__div__', '__rdiv__', # TODO: Is this different in py2 vs py3?
'__truediv__', '__rtruediv__',
'__floordiv__', '__rfloordiv__',
'__mod__', '__rmod__', '__divmod__',
'__pow__', '__rpow__'])
def test_generated_op_names(opname):
method = getattr(pd.Series, opname)
assert method.__name__ == opname
57 changes: 57 additions & 0 deletions pandas/tests/series/test_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,63 @@ def test_frame_sub_datetime64_not_ns(self):
Timedelta(days=2)])
tm.assert_frame_equal(res, expected)

def test_scalar_op_retains_name(self):
# GH#18964
ser = pd.Series([pd.Timedelta(days=1, hours=2)], name='NCC-1701D')

dt = pd.Timestamp(1968, 7, 8)
res = ser + dt
assert res.name == ser.name
res = dt - ser
assert res.name == ser.name

@pytest.mark.xfail(reason='GH#18963 DatetimeIndex+Series[datetime64] '
'returns DatetimeIndex with wrong name.')
def test_dti_op_retains_name(self):
# GH#18964
ser = pd.Series([pd.Timedelta(days=1, hours=2)], name='NCC-1701D')
dti = pd.DatetimeIndex([pd.Timestamp(1968, 7, 8)], name='Serenity')

expected = pd.Series([ser[0] + dti[0]], name=None)
res = ser + dti
tm.assert_series_equal(res, expected)
res = dti + ser # GH#18963 wrong type and name
tm.assert_series_equal(res, expected)

expected = pd.Series([dti[0] - ser[0]], name=None)
res = dti - ser # GH#18963 wrong type and name
tm.assert_series_equal(res, expected)
res = -ser + dti
tm.assert_series_equal(res, expected)

res = ser + dti.rename(name=ser.name)
tm.assert_series_equal(res, expected, check_names=False)
assert res.name == ser.name # TODO: Series.rename(name=...) fails

@pytest.mark.xfail(reason='GH#18824 Series[timedelta64]+TimedeltaIndex'
'gets name from TimedeltaIndex;'
'reverse op raises ValueError')
def test_tdi_op_retains_name(self):
# GH#18964
ser = pd.Series([pd.Timedelta(days=1, hours=2)], name='NCC-1701D')
tdi = pd.TimedeltaIndex(ser, name='Heart Of Gold')

expected = pd.Series([ser[0] + tdi[0]], name=None)
res = ser + tdi # right type, wrong name
tm.assert_series_equal(res, expected)
res = tdi + ser
tm.assert_series_equal(res, expected)

expected = pd.Series([ser[0] - tdi[0]], name=None)
res = ser - tdi # right type, wrong name
tm.assert_series_equal(res, expected)
res = -tdi + ser
tm.assert_series_equal(res, expected)

res = ser + tdi.rename(name=ser.name)
tm.assert_series_equal(res, expected, check_names=False)
assert res.name == ser.name # TODO: Series.rename(name=...) fails

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

Expand Down