Skip to content

WIP: decorator for ops boilerplate #24282

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 12 commits into from
Closed
5 changes: 3 additions & 2 deletions pandas/core/arrays/categorical.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from pandas.core.dtypes.inference import is_hashable
from pandas.core.dtypes.missing import isna, notna

from pandas.core import ops
from pandas.core.accessor import PandasDelegate, delegate_names
import pandas.core.algorithms as algorithms
from pandas.core.algorithms import factorize, take, take_1d, unique1d
Expand All @@ -53,14 +54,14 @@


def _cat_compare_op(op):

@ops.unpack_and_defer
def f(self, other):
# On python2, you can usually compare any type to any type, and
# Categoricals can be seen as a custom type, but having different
# results depending whether categories are the same or not is kind of
# insane, so be a bit stricter here and use the python3 idea of
# comparing only things of equal type.
if isinstance(other, ABCSeries):
return NotImplemented

if not self.ordered:
if op in ['__lt__', '__gt__', '__le__', '__ge__']:
Expand Down
4 changes: 4 additions & 0 deletions pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
def _make_comparison_op(cls, op):
# TODO: share code with indexes.base version? Main difference is that
# the block for MultiIndex was removed here.

# @ops.unpack_and_defer
Copy link
Member Author

Choose a reason for hiding this comment

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

Using this here leads to recursion errors, related to the fact that this only returns NotImplemented for ABCDataFrame. I think this will be easier to resolve after the change to composition.

def cmp_method(self, other):
if isinstance(other, ABCDataFrame):
return NotImplemented
Expand Down Expand Up @@ -885,6 +887,7 @@ def _time_shift(self, periods, freq=None):
return self._generate_range(start=start, end=end, periods=None,
freq=self.freq)

# @ops.unpack_and_defer
def __add__(self, other):
other = lib.item_from_zerodim(other)
if isinstance(other, (ABCSeries, ABCDataFrame)):
Expand Down Expand Up @@ -946,6 +949,7 @@ def __radd__(self, other):
# alias for __add__
return self.__add__(other)

# @ops.unpack_and_defer
def __sub__(self, other):
other = lib.item_from_zerodim(other)
if isinstance(other, (ABCSeries, ABCDataFrame)):
Expand Down
1 change: 1 addition & 0 deletions pandas/core/arrays/datetimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def _dt_array_cmp(cls, op):
opname = '__{name}__'.format(name=op.__name__)
nat_result = True if opname == '__ne__' else False

# @ops.unpack_and_defer
def wrapper(self, other):
meth = getattr(dtl.DatetimeLikeArrayMixin, opname)

Expand Down
18 changes: 6 additions & 12 deletions pandas/core/arrays/integer.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from pandas.core.dtypes.generic import ABCIndexClass, ABCSeries
from pandas.core.dtypes.missing import isna, notna

from pandas.core import nanops
from pandas.core import nanops, ops
from pandas.core.arrays import ExtensionArray, ExtensionOpsMixin


Expand Down Expand Up @@ -483,25 +483,17 @@ def _values_for_argsort(self):

@classmethod
def _create_comparison_method(cls, op):

# TODO: needs tests for deferring to DataFrame
@ops.unpack_and_defer
def cmp_method(self, other):

op_name = op.__name__
mask = None

if isinstance(other, (ABCSeries, ABCIndexClass)):
# Rely on pandas to unbox and dispatch to us.
return NotImplemented

if isinstance(other, IntegerArray):
other, mask = other._data, other._mask

elif is_list_like(other):
other = np.asarray(other)
if other.ndim > 0 and len(self) != len(other):
raise ValueError('Lengths must match to compare')

other = lib.item_from_zerodim(other)

# numpy will show a DeprecationWarning on invalid elementwise
# comparisons, this will raise in the future
with warnings.catch_warnings():
Expand Down Expand Up @@ -573,6 +565,8 @@ def _maybe_mask_result(self, result, mask, other, op_name):

@classmethod
def _create_arithmetic_method(cls, op):

# @ops.unpack_and_defer
Copy link
Member Author

Choose a reason for hiding this comment

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

@jreback using the decorator here breaks a bunch of tests, specifically ones operating with DataFrame. Why does this return NotImplemented for Series/Index but not DataFrame?

Copy link
Member Author

Choose a reason for hiding this comment

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

Below, why use other = other.item() instead of item_from_zerodim?

def integer_arithmetic_method(self, other):

op_name = op.__name__
Expand Down
1 change: 1 addition & 0 deletions pandas/core/arrays/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def _period_array_cmp(cls, op):
opname = '__{name}__'.format(name=op.__name__)
nat_result = True if opname == '__ne__' else False

# @ops.unpack_and_defer
def wrapper(self, other):
op = getattr(self.asi8, opname)
# We want to eventually defer to the Series or PeriodIndex (which will
Expand Down
23 changes: 7 additions & 16 deletions pandas/core/arrays/sparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
ABCIndexClass, ABCSeries, ABCSparseSeries)
from pandas.core.dtypes.missing import isna, na_value_for_dtype, notna

from pandas.core import ops
from pandas.core.accessor import PandasDelegate, delegate_names
import pandas.core.algorithms as algos
from pandas.core.arrays import ExtensionArray, ExtensionOpsMixin
Expand Down Expand Up @@ -1650,13 +1651,12 @@ def sparse_unary_method(self):

@classmethod
def _create_arithmetic_method(cls, op):

# TODO: needs test for deferring to DataFrame, op with 0-dim
@ops.unpack_and_defer
Copy link
Member Author

Choose a reason for hiding this comment

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

Both here and below this is technically a change since in the status quo this doesn't defer to DataFrame. Also doesn't call item_from_zerodim ATM.

def sparse_arithmetic_method(self, other):
op_name = op.__name__

if isinstance(other, (ABCSeries, ABCIndexClass)):
# Rely on pandas to dispatch to us.
return NotImplemented

if isinstance(other, SparseArray):
return _sparse_array_op(self, other, op, op_name)

Expand All @@ -1678,10 +1678,6 @@ def sparse_arithmetic_method(self, other):
with np.errstate(all='ignore'):
# TODO: delete sparse stuff in core/ops.py
# TODO: look into _wrap_result
if len(self) != len(other):
raise AssertionError(
("length mismatch: {self} vs. {other}".format(
self=len(self), other=len(other))))
if not isinstance(other, SparseArray):
dtype = getattr(other, 'dtype', None)
other = SparseArray(other, fill_value=self.fill_value,
Expand All @@ -1693,26 +1689,21 @@ def sparse_arithmetic_method(self, other):

@classmethod
def _create_comparison_method(cls, op):

# TODO: needs test for deferring to DataFrame, op with 0-dim
@ops.unpack_and_defer
def cmp_method(self, other):
op_name = op.__name__

if op_name in {'and_', 'or_'}:
op_name = op_name[:-1]

if isinstance(other, (ABCSeries, ABCIndexClass)):
# Rely on pandas to unbox and dispatch to us.
return NotImplemented

if not is_scalar(other) and not isinstance(other, type(self)):
# convert list-like to ndarray
other = np.asarray(other)

if isinstance(other, np.ndarray):
# TODO: make this more flexible than just ndarray...
if len(self) != len(other):
raise AssertionError("length mismatch: {self} vs. {other}"
.format(self=len(self),
other=len(other)))
other = SparseArray(other, fill_value=self.fill_value)

if isinstance(other, SparseArray):
Expand Down
89 changes: 15 additions & 74 deletions pandas/core/arrays/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@
is_integer_dtype, is_list_like, is_object_dtype, is_scalar,
is_string_dtype, is_timedelta64_dtype)
from pandas.core.dtypes.dtypes import DatetimeTZDtype
from pandas.core.dtypes.generic import (
ABCDataFrame, ABCIndexClass, ABCSeries, ABCTimedeltaIndex)
from pandas.core.dtypes.generic import ABCSeries, ABCTimedeltaIndex
from pandas.core.dtypes.missing import isna

from pandas.core import ops
Expand Down Expand Up @@ -74,6 +73,7 @@ def _td_array_cmp(cls, op):

meth = getattr(dtl.DatetimeLikeArrayMixin, opname)

# @ops.unpack_and_defer
def wrapper(self, other):
if _is_convertible_to_td(other) or other is NaT:
try:
Expand Down Expand Up @@ -302,11 +302,8 @@ def _addsub_offset_array(self, other, op):
raise TypeError("Cannot add/subtract non-tick DateOffset to {cls}"
.format(cls=type(self).__name__))

@ops.unpack_and_defer
def __mul__(self, other):
other = lib.item_from_zerodim(other)

if isinstance(other, (ABCDataFrame, ABCSeries, ABCIndexClass)):
return NotImplemented

if is_scalar(other):
# numpy will accept float and int, raise TypeError for others
Expand All @@ -316,14 +313,6 @@ def __mul__(self, other):
freq = self.freq * other
return type(self)(result, freq=freq)

if not hasattr(other, "dtype"):
# list, tuple
other = np.array(other)
if len(other) != len(self) and not is_timedelta64_dtype(other):
# Exclude timedelta64 here so we correctly raise TypeError
# for that instead of ValueError
raise ValueError("Cannot multiply with unequal lengths")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a small change (reflected in a test below). tdarr * tdarr[:-1] checks for length mismatch before checking for dtype compat, so now raises ValueError instead of TypeError.


if is_object_dtype(other):
# this multiplication will succeed only if all elements of other
# are int or float scalars, so we will end up with
Expand All @@ -338,12 +327,9 @@ def __mul__(self, other):

__rmul__ = __mul__

@ops.unpack_and_defer
def __truediv__(self, other):
# timedelta / X is well-defined for timedelta-like or numeric X
other = lib.item_from_zerodim(other)

if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)
Expand All @@ -365,14 +351,7 @@ def __truediv__(self, other):
freq = self.freq.delta / other
return type(self)(result, freq=freq)

if not hasattr(other, "dtype"):
# e.g. list, tuple
other = np.array(other)

if len(other) != len(self):
raise ValueError("Cannot divide vectors with unequal lengths")

elif is_timedelta64_dtype(other):
if is_timedelta64_dtype(other):
# let numpy handle it
return self._data / other

Expand All @@ -388,12 +367,9 @@ def __truediv__(self, other):
result = self._data / other
return type(self)(result)

@ops.unpack_and_defer
def __rtruediv__(self, other):
# X / timedelta is defined only for timedelta-like X
other = lib.item_from_zerodim(other)

if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)
Expand All @@ -411,14 +387,7 @@ def __rtruediv__(self, other):
.format(typ=type(other).__name__,
cls=type(self).__name__))

if not hasattr(other, "dtype"):
# e.g. list, tuple
other = np.array(other)

if len(other) != len(self):
raise ValueError("Cannot divide vectors with unequal lengths")

elif is_timedelta64_dtype(other):
if is_timedelta64_dtype(other):
# let numpy handle it
return other / self._data

Expand All @@ -438,11 +407,9 @@ def __rtruediv__(self, other):
__div__ = __truediv__
__rdiv__ = __rtruediv__

@ops.unpack_and_defer
def __floordiv__(self, other):
if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

other = lib.item_from_zerodim(other)
if is_scalar(other):
if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)
Expand All @@ -466,13 +433,7 @@ def __floordiv__(self, other):
freq = self.freq / other
return type(self)(result.view('m8[ns]'), freq=freq)

if not hasattr(other, "dtype"):
# list, tuple
other = np.array(other)
if len(other) != len(self):
raise ValueError("Cannot divide with unequal lengths")

elif is_timedelta64_dtype(other):
if is_timedelta64_dtype(other):
other = type(self)(other)

# numpy timedelta64 does not natively support floordiv, so operate
Expand Down Expand Up @@ -501,11 +462,9 @@ def __floordiv__(self, other):
raise TypeError("Cannot divide {typ} by {cls}"
.format(typ=dtype, cls=type(self).__name__))

@ops.unpack_and_defer
def __rfloordiv__(self, other):
if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

other = lib.item_from_zerodim(other)
if is_scalar(other):
if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)
Expand All @@ -523,13 +482,7 @@ def __rfloordiv__(self, other):
.format(typ=type(other).__name__,
cls=type(self).__name__))

if not hasattr(other, "dtype"):
# list, tuple
other = np.array(other)
if len(other) != len(self):
raise ValueError("Cannot divide with unequal lengths")

elif is_timedelta64_dtype(other):
if is_timedelta64_dtype(other):
other = type(self)(other)

# numpy timedelta64 does not natively support floordiv, so operate
Expand All @@ -551,45 +504,33 @@ def __rfloordiv__(self, other):
raise TypeError("Cannot divide {typ} by {cls}"
.format(typ=dtype, cls=type(self).__name__))

@ops.unpack_and_defer
def __mod__(self, other):
# Note: This is a naive implementation, can likely be optimized
if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

other = lib.item_from_zerodim(other)
if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)
return self - (self // other) * other

@ops.unpack_and_defer
def __rmod__(self, other):
# Note: This is a naive implementation, can likely be optimized
if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

other = lib.item_from_zerodim(other)
if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)
return other - (other // self) * self

@ops.unpack_and_defer
def __divmod__(self, other):
# Note: This is a naive implementation, can likely be optimized
if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

other = lib.item_from_zerodim(other)
if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)

res1 = self // other
res2 = self - res1 * other
return res1, res2

@ops.unpack_and_defer
def __rdivmod__(self, other):
# Note: This is a naive implementation, can likely be optimized
if isinstance(other, (ABCSeries, ABCDataFrame, ABCIndexClass)):
return NotImplemented

other = lib.item_from_zerodim(other)
if isinstance(other, (timedelta, np.timedelta64, Tick)):
other = Timedelta(other)

Expand Down
Loading