-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: fix issue of boolean comparison on empty DataFrames (GH5808) #5810
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
@jtratner I'd like you to look at this I actually think my 'fix' is wrong, as it patches an arithmetic methods, but maybe this is the case of that you have here: https://github.com/pydata/pandas/blob/master/pandas/core/ops.py#L223 e.g. 'rand_' is called (I think because it is trying the reverse ops first?) |
@jtratner if you have a chance... |
@jreback sorry missed this I'll take a look now. |
It's sort of self-documented (though not really well) which method does what in ops: frame_special_funcs = dict(arith_method=_arith_method_FRAME,
radd_func=_radd_compat,
comp_method=_comp_method_FRAME,
bool_method=_arith_method_FRAME) and frame flex funcs don't include the bool methods... frame never had a separate bool method. Ignoring TypeErrors there feels wrong - wouldn't it silence other actually useful TypeErrrors? Why couldn't we just check whether either or both are empty and then skip the transformation op if so? The reason that I popped the |
ok so for Series we have a for Frame I guess didn't exist before and is handled by arithmetic methods. Biggest problem is I cannot find ANY tests here with boolean on frames. I guess I can just handle the empties. |
Problem is have all kinds of weirdness if these are treated as arithmetic operators these should all raise, but an int and bool are ok. I think the dtypes have to be restricted for this to properly work.
|
I agree, it's totally weird, but that's what we had previously. So do you want to just coerce it to bool first or are you going to restrict it to bool dtype only? |
I think I need to add |
that makes sense, that should simplify things a bit - though still need to mask it. |
no perfect but preserves functionaility (and handles the empty case) |
current format looks better, |
BUG: fix issue of boolean comparison on empty DataFrames (GH5808)
closes #5808