-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Closed
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
65dd8b3
remove unused code, impossible cases. add todo comments
jbrockmendel f65eab4
add tests for op names
jbrockmendel 9091386
Merge branch 'master' of https://github.com/pandas-dev/pandas into op…
jbrockmendel 5a5d34a
trim the set of tested funcnames
jbrockmendel 38d6c08
troubleshooting, add tests that belong elsewhere
jbrockmendel c712bfc
fix Series op signatures
jbrockmendel 4a2a021
cleanup commented-out bits from troubleshooting
jbrockmendel e53b7fa
cleanup leftover troubleshooting relics
jbrockmendel 13ebe0a
Merge branch 'master' of https://github.com/pandas-dev/pandas into op…
jbrockmendel f37eb9d
Merge branch 'master' of https://github.com/pandas-dev/pandas into op…
jbrockmendel 58f7ef7
Merge branch 'master' of https://github.com/pandas-dev/pandas into op…
jbrockmendel d8305c1
Merge branch 'master' of https://github.com/pandas-dev/pandas into op…
jbrockmendel fdf461e
requested edits
jbrockmendel 5b704fa
Merge branch 'master' of https://github.com/pandas-dev/pandas into op…
jbrockmendel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(): | ||
|
@@ -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) | ||
|
||
|
@@ -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 | ||
assert self.is_timedelta_lhs or self.is_datetime_lhs | ||
|
||
# right | ||
self.is_offset_rhs = is_offsetlike(right) | ||
|
@@ -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__'): | ||
|
@@ -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 | ||
|
@@ -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] | ||
|
@@ -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: | ||
|
@@ -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: | ||
|
||
|
@@ -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) | ||
|
||
|
@@ -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__')): | ||
|
@@ -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 | ||
|
@@ -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 | ||
|
||
|
||
|
@@ -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) | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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): | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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