-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG: Make bool(str_scalar) and str_scalar.astype(bool) consistent #9848
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
5cfbb51
to
baefb91
Compare
``string.astype(bool)`` now uses the normal python semantics for converting | ||
--------------------------------------------------------------------------- | ||
Previously this was interpreted as ``string.astype(int).astype(bool)``, which | ||
interpeted ``'0'`` as ``False``. |
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.
Do we want to go through a FutureWarning
here?
baefb91
to
10e1ff5
Compare
Failures seem real. Not sure I understand why |
10e1ff5
to
c1e56a4
Compare
I wish |
Another way the old behavior was bad: |
This seems like a fairly clear "good move", and code looks fine to me. As to whether we want a round of deprecation with FutureWarning: I lean towards yes, since it's not a very critical bug, yet the change is quite user-visible and I can even imagine someone depending on it. So I imagine a FutureWarning for 1.14, then change the behavior in 1.15. |
>>> np.array([True, False], dtype=bool).astype(str).astype(bool)
Edit: (Wait never mind, this errors out in current master) |
A naive implementation of adding the following to
\leads to the warning being emitted once for every single element in the array!
Should I only emit the warning if the behaviour will change? That's still pathological for |
c1e56a4
to
c81c9f9
Compare
Needs rebase, also to get the merged #9856. |
Caused by string values being cast to an int first and then a bool, ie arr.astype(int).astype(bool). This causes crashes on non-numeric strings, and only accepts strings of 0 as false. Fixes numpy#9847
c81c9f9
to
6e1d637
Compare
Rebased, but not ready to merge without a decision about if and when to |
Closing in favor of having discussion in #9875, since this is a wider issue than anticipated |
Fixes #9847, by changing
str_scalar.astype(bool)
to no longer meanbool(int(str_scalar))