-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN/REF: Unify Arithmetic Methods #27413
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
Conversation
Looks quite nice, thanks. I think @jreback is fixing the numpy dev CI failure. |
Didn't look at the actual PR, but just to note: I think we should stop merging such cleaning / refactoring PRs before 0.25 (or already branch). Which goes for most of Brock's PRs (nothing personal, Brock, it is just that you are doing useful stuff that touches existing code ;)) |
well then we should branch / release. These are + PRs and to be honest if all of the tests pass I don't see a reason to not merge. |
+1 |
thanks @jbrockmendel nice code removal. |
2 goals here.
First is to end up with a single implementation for our arithmetic ops. By implementing the missing methods directly on DTA/PA, we make it so we don't need to special-case inside the
Index.__foo__
methods, so can dispatch to Series more directly.Second is to get the wrapper defined in_arith_method_SERIES to the point where we can use it block-wise instead of column-wise, to address the recent perf issues. This makes progress towards that goal by tightening up the allowed types in masked_arith_op
Oh, and we also get rid of a usage of values_from_object, which is adjacent to #27165 and #27167