-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
API: ops alignment behavior inconsistencies #28759
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
Comments
The (non-)alignment of comparison ops (for the first two cases) is something we discussed long time ago, I recall, and was at that time decided consciously. To be clear: not saying that it is therefore the best behaviour, just meaning it is not an accidental, historical inconsistency, as far as I recall. So worth digging up the reasoning (will try to look for relevant issues later). |
@jorisvandenbossche thanks. As long as its intentional, its fine by me. If you do stumble on the old thread, pls LMK and I'll add a comment in the code pointing back to it for the next time I forget. |
actually i would like to see case 3)’have the non-align for comparisons (raise) |
So #36795 fixed the inconsistency for df/ser comparisons compared to the others. Shall we close this then? |
In both Series and DataFrame ops, we have inconsistent behavior for when we call
self.align
vs when we raise. Everything discussed here is for non-flex ops.Case 1: consider
op(ser1, ser2)
for two Series with non-matching indexes.self.align(other)
ValueError("Can only compare identically-labeled Series objects")
self.align(other)
Case 2: consider
op(df1, df2)
for two DataFrames with non-matching axesself.align(other)
ValueError("Can only compare identically-labeled DataFrame objects")
self.align(other)
Case 3) consider
op(df, ser)
. This always aligns, with comparison not being treated differently from the other two.The policy (and code) would be simpler if we changed this so that either:
a) the comparison op in case 3 doesn't align, matching cases 1 and 2
b) comparison ops always align, matching arithmetic and logical ops
The text was updated successfully, but these errors were encountered: