Skip to content

BUG: Fixes #7395, operator.index now fails on numpy.bool_#9685

Merged
charris merged 4 commits into
numpy:masterfrom
eric-wieser:bool-__index__
Oct 21, 2017
Merged

BUG: Fixes #7395, operator.index now fails on numpy.bool_#9685
charris merged 4 commits into
numpy:masterfrom
eric-wieser:bool-__index__

Conversation

@eric-wieser

Copy link
Copy Markdown
Member

Rebase of @emilienkofman's #7420, along with added test and release notes. The description from there is

Attempt to fix the bug described in #7395. If this needs a depreciation warning I can change the Err to Warning and maybe add a test (?)

@mhvk mhvk left a comment

Copy link
Copy Markdown
Contributor

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,

Copy link
Copy Markdown
Contributor

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).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good catch

@eric-wieser

Copy link
Copy Markdown
Member Author

@mhvk: Addressed. Does this want a deprecation cycle? We already hit operator.index(array(True)) in a previous release

@seberg

seberg commented Sep 17, 2017

Copy link
Copy Markdown
Member

Unless this is a big bug somewhere, the default should be to go through the deprecation I think.

@eric-wieser

Copy link
Copy Markdown
Member Author

@seberg: Changed to a deprecation

@charris charris merged commit b3f72f8 into numpy:master Oct 21, 2017
@charris

charris commented Oct 21, 2017

Copy link
Copy Markdown
Member

Fixed merge conflict and merged. Thanks Eric.

BvB93 pushed a commit to BvB93/numpy that referenced this pull request Sep 7, 2020
It's been deprecated since numpy#9685
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants