-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
De-duplicate dispatch code, remove unreachable branches #22068
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
Changes from 5 commits
a13c161
c8948bc
2f25223
04213ce
8527bd2
92df36d
2ae4b2f
94f168a
f5bb3b6
c36e672
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1114,6 +1114,7 @@ def na_op(x, y): | |
result[mask] = op(x[mask], com.values_from_object(y[mask])) | ||
else: | ||
assert isinstance(x, np.ndarray) | ||
assert lib.is_scalar(y) | ||
result = np.empty(len(x), dtype=x.dtype) | ||
mask = notna(x) | ||
result[mask] = op(x[mask], y) | ||
|
@@ -1160,6 +1161,7 @@ def wrapper(left, right): | |
|
||
elif (is_extension_array_dtype(left) or | ||
is_extension_array_dtype(right)): | ||
# TODO: should this include `not is_scalar(right)`? | ||
return dispatch_to_extension_op(op, left, right) | ||
|
||
elif is_datetime64_dtype(left) or is_datetime64tz_dtype(left): | ||
|
@@ -1249,13 +1251,11 @@ def na_op(x, y): | |
# should have guarantess on what x, y can be type-wise | ||
# Extension Dtypes are not called here | ||
|
||
# dispatch to the categorical if we have a categorical | ||
# in either operand | ||
if is_categorical_dtype(y) and not is_scalar(y): | ||
# The `not is_scalar(y)` check excludes the string "category" | ||
return op(y, x) | ||
# Checking that cases that were once handled here are no longer | ||
# reachable. | ||
assert not (is_categorical_dtype(y) and not is_scalar(y)) | ||
|
||
elif is_object_dtype(x.dtype): | ||
if is_object_dtype(x.dtype): | ||
result = _comp_method_OBJECT_ARRAY(op, x, y) | ||
|
||
elif is_datetimelike_v_numeric(x, y): | ||
|
@@ -1313,7 +1313,7 @@ def wrapper(self, other, axis=None): | |
return self._constructor(res_values, index=self.index, | ||
name=res_name) | ||
|
||
if is_datetime64_dtype(self) or is_datetime64tz_dtype(self): | ||
elif is_datetime64_dtype(self) or is_datetime64tz_dtype(self): | ||
# Dispatch to DatetimeIndex to ensure identical | ||
# Series/Index behavior | ||
if (isinstance(other, datetime.date) and | ||
|
@@ -1355,8 +1355,9 @@ def wrapper(self, other, axis=None): | |
name=res_name) | ||
|
||
elif (is_extension_array_dtype(self) or | ||
(is_extension_array_dtype(other) and | ||
not is_scalar(other))): | ||
(is_extension_array_dtype(other) and not is_scalar(other))): | ||
# Note: the `not is_scalar(other)` condition rules out | ||
# e.g. other == "category" | ||
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. the issue here is we need to be able to distinguish between an actual dtype comparison and a real comparison, e.g. this is pretty thorny, e.g. how do you know when to convert a scalar string in a comparison op to an actual dtype for comparisons 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. Yah. A while ago there was a check |
||
return dispatch_to_extension_op(op, self, other) | ||
|
||
elif isinstance(other, ABCSeries): | ||
|
@@ -1379,13 +1380,6 @@ def wrapper(self, other, axis=None): | |
# is not. | ||
return result.__finalize__(self).rename(res_name) | ||
|
||
elif isinstance(other, pd.Categorical): | ||
# ordering of checks matters; by this point we know | ||
# that not is_categorical_dtype(self) | ||
res_values = op(self.values, other) | ||
return self._constructor(res_values, index=self.index, | ||
name=res_name) | ||
|
||
elif is_scalar(other) and isna(other): | ||
# numpy does not like comparisons vs None | ||
if op is operator.ne: | ||
|
@@ -1515,6 +1509,41 @@ def flex_wrapper(self, other, level=None, fill_value=None, axis=0): | |
# ----------------------------------------------------------------------------- | ||
# DataFrame | ||
|
||
def dispatch_to_series(left, right, func): | ||
""" | ||
Evaluate the frame operation func(left, right) by evaluating | ||
column-by-column, dispatching to the Series implementation. | ||
|
||
Parameters | ||
---------- | ||
left : DataFrame | ||
right : scalar or DataFrame | ||
func : arithmetic or comparison operator | ||
|
||
Returns | ||
------- | ||
DataFrame | ||
""" | ||
# Note: we use iloc to access columns for compat with cases | ||
# with non-unique columns. | ||
if lib.is_scalar(right): | ||
new_data = {i: func(left.iloc[:, i], right) | ||
for i in range(len(left.columns))} | ||
elif isinstance(right, ABCDataFrame): | ||
assert right._indexed_same(left) | ||
new_data = {i: func(left.iloc[:, i], right.iloc[:, i]) | ||
for i in range(len(left.columns))} | ||
else: | ||
# Remaining cases have less-obvious dispatch rules | ||
raise NotImplementedError | ||
|
||
result = left._constructor(new_data, index=left.index, copy=False) | ||
# Pin columns instead of passing to constructor for compat with | ||
# non-unique columns case | ||
result.columns = left.columns | ||
return result | ||
|
||
|
||
def _combine_series_frame(self, other, func, fill_value=None, axis=None, | ||
level=None, try_cast=True): | ||
""" | ||
|
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.
prob should just import at the top