-
-
Notifications
You must be signed in to change notification settings - Fork 12k
BUG: Fixes #7395, operator.index now fails on numpy.bool_ #9685
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
mhvk
left a comment
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.
Wondering about the one comment; less code is better...
| bool_index(PyObject *a) | ||
| { | ||
| return PyInt_FromLong(PyArrayScalar_VAL(a, Bool)); | ||
| PyErr_SetString(PyExc_TypeError, |
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.
Instead of raising an error here, isn't it easier to remove this whole definition and not define (or explicitly set to NULL; not sure) PyBoolArrType_Type.tp_as_number->nb_index? (it is currently defined in L4114).
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.
Yeah, good catch
b6760ba to
94d0b68
Compare
|
@mhvk: Addressed. Does this want a deprecation cycle? We already hit |
|
Unless this is a big bug somewhere, the default should be to go through the deprecation I think. |
94d0b68 to
04b14d6
Compare
|
@seberg: Changed to a deprecation |
|
Fixed merge conflict and merged. Thanks Eric. |
It's been deprecated since numpy#9685
Rebase of @emilienkofman's #7420, along with added test and release notes. The description from there is