8000 MAINT: Make the refactor suggested in prepare_index by eric-wieser · Pull Request #8278 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

MAINT: Make the refactor suggested in prepare_index #8278

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Jul 18, 2017
Merged

MAINT: Make the refactor suggested in prepare_index #8278

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
87e1294
MAINT: Make the refactor suggested in prepare_index
eric-wieser Nov 14, 2016
bfd70c2
MAINT: Fix typo, remove unnecessary nesting
eric-wieser Dec 5, 2016
37e6e1f
MAINT: use PyTuple_Pack, which is probably faster than Py_BuildValue
eric-wieser Dec 21, 2016
3a0ad85
MAINT: Overhaul function to try and increase speed
eric-wieser Apr 7, 2017
8c4e556
BUG: Fix reference counting
eric-wieser Apr 9, 2017
5d9315c
STY: Fix comment capitalization and format
eric-wieser Apr 9, 2017
966b2c7
fixup! BUG: Fix reference counting
eric-wieser Apr 9, 2017
9832f05
fixup! MAINT: Overhaul function to try and increase speed
eric-wieser Apr 9, 2017
68bad6a
MAINT: Improve comments, simplify handling of tuple subclasses
eric-wieser Jul 16, 2017
c587963
fixup! MAINT: Improve comments, simplify handling of tuple subclasses
eric-wieser Jul 16, 2017
105e0b4
fixup! MAINT: Improve comments, simplify handling of tuple subclasses
eric-wieser Jul 16, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
BUG: Fix reference counting
  • Loading branch information
@eric-wieser
eric-wieser committed Apr 9, 2017
commit 8c4e5569ab62527d434e242d6bee57809d74323c
29 changes: 24 additions & 5 deletions numpy/core/src/multiarray/mapping.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ PyArray_MapIterSwapAxes(PyArrayMapIterObject *mit, PyArrayObject **ret, int getm
*ret = (PyArrayObject *)new;
}

NPY_NO_EXPORT NPY_INLINE void
multi_DECREF(PyObject **objects, npy_intp n)
Copy link
Member

Choose a reason for hiding this comment

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

First wasn't sure I like this, but it seems harmless :).

{
npy_intp i;
for (i = 0; i < n; i++) {
Py_DECREF(objects[i]);
}
}

/**
* Turn an index argument into a c-array of `PyObject *`s, one for each index.
*
Expand All @@ -158,7 +167,7 @@ PyArray_MapIterSwapAxes(PyArrayMapIterObject *mit, PyArrayObject **ret, int getm
* @param index The index object, which may or may not be a tuple. This is a
* borrowed reference.
* @param result An empty buffer of 2*NPY_MAXDIMS PyObject* to write each
* index component to. The references written are borrowed.
* index component to. The references written are new..
* This function will in some cases clobber the array beyond
Copy link
Contributor

Choose a reason for hiding this comment

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

The comments are very helpful, but here maybe be even more explicit and say "beyond the number of items returned".

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do - I was struggling to phrase this

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I've removed that remark entirely, as it made more sense under the description of the return value:

 * @returns        The number of items in `result`, or -1 if an error occured.
 *                 The entries in `result` at and beyond this index should be
 *                 assumed to contain garbage, even if they were initialized
 *                 to NULL, so are not safe to Py_XDECREF.

* the last item unpacked.
*
Expand All @@ -180,6 +189,7 @@ unpack_indices(PyObject *index, PyObject *result[2*NPY_MAXDIMS])
}
for (i = 0; i < n; i++) {
result[i] = PyTuple_GET_ITEM(index, i);
Py_INCREF(result[i]);
}
return n;
}
Expand All @@ -196,6 +206,7 @@ unpack_indices(PyObject *index, PyObject *result[2*NPY_MAXDIMS])
|| PyArray_Check(index)
|| !PySequence_Check(index)) {

Py_INCREF(index);
result[0] = index;
return 1;
}
Expand All @@ -214,8 +225,10 @@ unpack_indices(PyObject *index, PyObject *result[2*NPY_MAXDIMS])
for (i = 0; i < n; i++) {
result[i] = PySequence_GetItem(index, i);
if (result[i] == NULL) {
multi_DECREF(result, i);
return -1;
}
Py_INCREF(result[i]);
}
return n;
}
Expand All @@ -233,13 +246,15 @@ unpack_indices(PyObject *index, PyObject *result[2*NPY_MAXDIMS])
n = PySequence_Size(index);
if (n < 0) {
PyErr_Clear();
Py_INCREF(index);
result[0] = index;
return 1;
}

/* for some reason, anything that's long but not too long is turned into
* a single index. The *2 is missing here for backward-compatibility. */
if (n >= NPY_MAXDIMS) {
Py_INCREF(index);
result[0] = index;
return 1;
}
Expand All @@ -253,13 +268,15 @@ unpack_indices(PyObject *index, PyObject *result[2*NPY_MAXDIMS])
if (commit_to_unpack) {
/* propagate errors */
if (tmp_obj == NULL) {
multi_DECREF(result, i);
return -1;
}
}
else {
/* if getitem fails (unusual) before we've committed, then
* commit to not unpacking */
if (tmp_obj == NULL) {
multi_DECREF(result, i);
PyErr_Clear();
break;
}
Expand All @@ -273,18 +290,18 @@ unpack_indices(PyObject *index, PyObject *result[2*NPY_MAXDIMS])
commit_to_unpack = 1;
}
}

Py_DECREF(tmp_obj);
}

/* unpacking was the right thing to do, and we already did it */
if (commit_to_unpack) {
return n;
}

/* got to the end, never found an indication that we should have unpacked */
else {
/* we already filled result, but it doesn't matter */
/* we already filled result, so empty it first */
multi_DECREF(result, n);

Py_INCREF(index);
result[0] = index;
return 1;
}
Expand Down Expand Up @@ -757,13 +774,15 @@ prepare_index(PyArrayObject *self, PyObject *index,
*ndim = new_ndim + fancy_ndim;
*out_fancy_ndim = fancy_ndim;

multi_DECREF(raw_indices, index_ndim);

return index_type;

failed_building_indices:
for (i=0; i < curr_idx; i++) {
Py_XDECREF(indices[i].object);
}
multi_DECREF(raw_indices, index_ndim);
return -1;
}

Expand Down
0