Skip to content

MAINT: BUG: review need for NpyIter_Close, add missing test for out #11370

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

Closed
wants to merge 1 commit into from
Closed

MAINT: BUG: review need for NpyIter_Close, add missing test for out #11370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions numpy/core/src/multiarray/_multiarray_tests.c.src
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1034,9 +1034,11 @@ test_nditer_too_large(PyObject *NPY_UNUSED(self), PyObject *args) {
break;
}

NpyIter_Close(iter);
NpyIter_Deallocate(iter);
Py_RETURN_NONE;
fail:
NpyIter_Close(iter);
NpyIter_Deallocate(iter);
return NULL;
}
Expand Down
2 changes: 2 additions & 0 deletions numpy/core/src/multiarray/compiled_base.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,7 @@ arr_ravel_multi_index(PyObject *self, PyObject *args, PyObject *kwds)
Py_XDECREF(op[i]);
}
npy_free_cache_dim_obj(dimensions);
/* no NpyIter_Close required since output is always allocated */
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason not to just always call it? Testing a handful of flags should be pretty cheap, and if its not, you could cache them in the constructor?

Copy link
Member Author

Choose a reason for hiding this comment

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

We store the need to resolve the writeback in a per-operand flag, so traversing those would be very cheap.

This PR is more to check if our testing is adequate, if there were places that are missing a needed NpyIter_Close that would indicate missing tests. It seems that part is OK, barring mistakes.

NpyIter_Deallocate(iter);
return NULL;
}
Expand Down Expand Up @@ -1373,6 +1374,7 @@ arr_unravel_index(PyObject *self, PyObject *args, PyObject *kwds)
Py_DECREF(ret_arr);
Py_XDECREF(indices);
npy_free_cache_dim_obj(dimensions);
/* no NpyIter_Close since iter is NPY_ITER_READONLY */
NpyIter_Deallocate(iter);

return ret_tuple;
Expand Down
7 changes: 7 additions & 0 deletions numpy/core/src/multiarray/ctors.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2794,6 +2794,7 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
NPY_NO_CASTING,
NULL);
if (src_iter == NULL) {
/* no NpyIter_Close since iter is created with NPY_NO_CASTING */
NpyIter_Deallocate(dst_iter);
return -1;
}
Expand All @@ -2813,7 +2814,9 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
src_itemsize = PyArray_DESCR(src)->elsize;

if (dst_iternext == NULL || src_iternext == NULL) {
/* no NpyIter_Close since iter is created with NPY_NO_CASTING */
NpyIter_Deallocate(dst_iter);
/* no NpyIter_Close since src_iter is NPY_ITER_READONLY */
NpyIter_Deallocate(src_iter);
return -1;
}
Expand All @@ -2834,7 +2837,9 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
0,
&stransfer, &transferdata,
&needs_api) != NPY_SUCCEED) {
/* no NpyIter_Close since iter is created with NPY_NO_CASTING */
NpyIter_Deallocate(dst_iter);
/* no NpyIter_Close since src_iter is NPY_ITER_READONLY */
NpyIter_Deallocate(src_iter);
return -1;
}
Expand Down Expand Up @@ -2884,7 +2889,9 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
NPY_END_THREADS;

NPY_AUXDATA_FREE(transferdata);
/* no NpyIter_Close since iter is created with NPY_NO_CASTING */
NpyIter_Deallocate(dst_iter);
/* no NpyIter_Close since src_iter is NPY_ITER_READONLY */
NpyIter_Deallocate(src_iter);

return PyErr_Occurred() ? -1 : 0;
Expand Down
3 changes: 3 additions & 0 deletions numpy/core/src/multiarray/datetime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3519,6 +3519,7 @@ find_string_array_datetime64_type(PyArrayObject *arr,

iternext = NpyIter_GetIterNext(iter, NULL);
if (iternext == NULL) {
/* no NpyIter_Close since iter is NPY_ITER_READONLY */
NpyIter_Deallocate(iter);
return -1;
}
Expand All @@ -3533,6 +3534,7 @@ find_string_array_datetime64_type(PyArrayObject *arr,
tmp_buffer = PyArray_malloc(maxlen+1);
if (tmp_buffer == NULL) {
PyErr_NoMemory();
/* no NpyIter_Close since iter is NPY_ITER_READONLY */
NpyIter_Deallocate(iter);
return -1;
}
Expand Down Expand Up @@ -3585,6 +3587,7 @@ find_string_array_datetime64_type(PyArrayObject *arr,
} while(iternext(iter));

PyArray_free(tmp_buffer);
/* no NpyIter_Close since iter is NPY_ITER_READONLY */
NpyIter_Deallocate(iter);

return 0;
Expand Down
5 changes: 5 additions & 0 deletions numpy/core/src/multiarray/datetime_busday.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,8 @@ business_day_offset(PyArrayObject *dates, PyArrayObject *offsets,
Py_XDECREF(dtypes[1]);
Py_XDECREF(dtypes[2]);
if (iter != NULL) {
/* not needed since iter is created with NPY_SAFE_CASTING */
/* NpyIter_Close(iter); */
if (NpyIter_Deallocate(iter) != NPY_SUCCEED) {
Py_XDECREF(ret);
ret = NULL;
Expand Down Expand Up @@ -697,6 +699,8 @@ business_day_count(PyArrayObject *dates_begin, PyArrayObject *dates_end,
Py_XDECREF(dtypes[1]);
Py_XDECREF(dtypes[2]);
if (iter != NULL) {
/* not needed since iter is created with NPY_SAFE_CASTING */
/* NpyIter_Close(iter); */
if (NpyIter_Deallocate(iter) != NPY_SUCCEED) {
Py_XDECREF(ret);
ret = NULL;
Expand Down Expand Up @@ -822,6 +826,7 @@ is_business_day(PyArrayObject *dates, PyArrayObject *out,
Py_XDECREF(dtypes[0]);
Py_XDECREF(dtypes[1]);
if (iter != NULL) {
NpyIter_Close(iter);
if (NpyIter_Deallocate(iter) != NPY_SUCCEED) {
Py_XDECREF(ret);
ret = NULL;
Expand Down
1 change: 1 addition & 0 deletions numpy/core/src/multiarray/datetime_strings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,7 @@ array_datetime_as_string(PyObject *NPY_UNUSED(self), PyObject *args,
Py_XDECREF(op_dtypes[0]);
Py_XDECREF(op_dtypes[1]);
if (iter != NULL) {
/* no NpyIter_Close needed since writeable output is freshly created */
NpyIter_Deallocate(iter);
}

Expand Down
5 changes: 5 additions & 0 deletions numpy/core/src/multiarray/item_selection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2133,6 +2133,7 @@ PyArray_CountNonzero(PyArrayObject *self)
/* Get the pointers for inner loop iteration */
iternext = NpyIter_GetIterNext(iter, NULL);
if (iternext == NULL) {
/* no NpyIter_Close since iter has NPY_ITER_READONLY */
NpyIter_Deallocate(iter);
return -1;
}
Expand Down Expand Up @@ -2165,6 +2166,7 @@ PyArray_CountNonzero(PyArrayObject *self)
finish:
NPY_END_THREADS;

/* no NpyIter_Close since iter has NPY_ITER_READONLY */
NpyIter_Deallocate(iter);

return nonzero_count;
Expand Down Expand Up @@ -2291,12 +2293,14 @@ PyArray_Nonzero(PyArrayObject *self)
/* Get the pointers for inner loop iteration */
iternext = NpyIter_GetIterNext(iter, NULL);
if (iternext == NULL) {
/* no NpyIter_Close since iter has NPY_ITER_READONLY */
NpyIter_Deallocate(iter);
Py_DECREF(ret);
return NULL;
}
get_multi_index = NpyIter_GetGetMultiIndex(iter, NULL);
if (get_multi_index == NULL) {
/* no NpyIter_Close since iter has NPY_ITER_READONLY */
NpyIter_Deallocate(iter);
Py_DECREF(ret);
return NULL;
Expand Down Expand Up @@ -2330,6 +2334,7 @@ PyArray_Nonzero(PyArrayObject *self)
NPY_END_THREADS;
}

/* no NpyIter_Close since iter has NPY_ITER_READONLY */
NpyIter_Deallocate(iter);

finish:
Expand Down
6 changes: 6 additions & 0 deletions numpy/core/tests/test_datetime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1698,6 +1698,12 @@ def test_datetime_busday_offset(self):
assert_equal(np.busday_offset(np.datetime64('NaT'), 1, roll='preceding'),
np.datetime64('NaT'))

# Use out
out = np.array(['2006-02-01'], dtype='datetime64[ms]')
a = np.array(['2006-02-01'], dtype='datetime64[D]')
res = np.busday_offset(a, 25.0, out=out)
assert_equal(res, out)
assert_equal(res[0], np.datetime64('2006-03-08'))

def test_datetime_busdaycalendar(self):
# Check that it removes NaT, duplicates, and weekends
Expand Down