-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
PERF: do DataFrame.op(series, axis=0) blockwise #31296
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
PERF: do DataFrame.op(series, axis=0) blockwise #31296
Conversation
pandas/core/ops/array_ops.py
Outdated
@@ -200,6 +213,27 @@ def arithmetic_op( | |||
return res_values | |||
|
|||
|
|||
def _broadcast_comparison_op(lvalues, rvalues, op): |
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.
(Brief) docstring?
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.
can you add this (and types if you can)
@@ -200,6 +213,27 @@ def arithmetic_op( | |||
return res_values | |||
|
|||
|
|||
def _broadcast_comparison_op(lvalues, rvalues, op): | |||
if isinstance(rvalues, np.ndarray): |
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.
this looks like the case on L81 in array_ops.py
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.
can you follow up with consolidate if this is the case
…rf-arith-series-col
…rf-arith-series-col
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.
looks good, pls rebase. i need to go over again.
…rf-arith-series-col
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.
looks fine. rebase. I think this needs the other branch that pushes down str_rep (may have merged already), if not (or already merged), then lgtm. ping on green.
pandas/core/ops/array_ops.py
Outdated
@@ -200,6 +213,27 @@ def arithmetic_op( | |||
return res_values | |||
|
|||
|
|||
def _broadcast_comparison_op(lvalues, rvalues, op): |
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.
can you add this (and types if you can)
|
||
|
||
def _can_broadcast(lvalues, rvalues) -> bool: | ||
# We assume that lengths dont match |
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.
same
add a whatsnew in perf section |
…rf-arith-series-col
…rf-arith-series-col
…rf-arith-series-col
…rf-arith-series-col
done |
…rf-arith-series-col
…rf-arith-series-col
…rf-arith-series-col
…rf-arith-series-col
rebased+green |
is the github author thing fixed? |
not that im aware of |
GH thing is fixed now i think |
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.
ok, followon comment
@@ -200,6 +213,27 @@ def arithmetic_op( | |||
return res_values | |||
|
|||
|
|||
def _broadcast_comparison_op(lvalues, rvalues, op): | |||
if isinstance(rvalues, np.ndarray): |
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.
can you follow up with consolidate if this is the case
thanks |
Also fixes the same bug as #31271 (with the same test ported), so if this is accepted that will be closeable.
~2000x speedups for very-wide mixed-dtype cases.