-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
WRN: let + * and - pass thru on boolean with a warning #7245
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
if op_str in deprecated: | ||
warnings.warn("operator %r is deprecated for bool dtype" | ||
", use %r instead" % (op_str, deprecated[op_str]), | ||
FutureWarning) |
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.
maybe make it a UserWarning
as its not deprecated per se, just not supported really ?
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
|
ah right ... that reminds me i actually need to test this with numexpr which was the whole point! numexpr doesn't support boolean arithmetic, in this case that's what I mean when I say not supported |
I thought u were going to do a warning, then just translate the symbols and do it anyhow (e.g. '*' -> '&') etc |
ah, I thought you were going to detect this and then just use python evaluation and not numexpr (in that sense my comment above). What I mean is: |
i see @jreback i misunderstood what you meant by translation...that's actually probably the simplest method |
yes I agree with @jorisvandenbossche (that's even better), but I would still do the warning; you can just bail out of the evaluation and 'fallback' to python eval |
ok so translate or no? very simple and would keep the performance for those ops (not that they are really used that much) |
@cpcloud I think just fallback as @jorisvandenbossche suggests (with a warning) |
ok |
how about 'operator "BLAH" not supported by numexpr for bool dtype, use "BLARG" instead'? |
still not totally clear that it works tho |
@jorisvandenbossche ok with the latest message? |
ok! |
Looks good to me need to adjust the release notes / v0.14.0.txt to reflect this (and add this issue number too).
|
But give a warning suggesting a better way to do it
@jreback good on the rls notes? |
yep |
WRN: let + * and - pass thru on boolean with a warning
closes #7210