numpy / numpy

The fundamental package for scientific computing with Python.
https://numpy.org
Other
27.97k stars 10.05k forks source link

Ticket 1588 #398

Closed certik closed 12 years ago

certik commented 12 years ago

This issue is for the ticket:

http://projects.scipy.org/numpy/ticket/1588

Because the trac server is unreliable (frequently fails and I need to wait couple minutes for the database to be unlocked again --- see my email to the numpy list) and I need to keep track of my progress.

Summary of the problem

The following patch fixes it, but probably creates a leak sometimes:

diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c
index 618a871..a360c4e 100644
--- a/numpy/core/src/multiarray/calculation.c
+++ b/numpy/core/src/multiarray/calculation.c
@@ -981,7 +981,7 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
     }
     else {
         /* Side-effect of PyArray_FromAny */
-        Py_DECREF(indescr);
+        //Py_DECREF(indescr);
     }

     /*

This code was first introduced by 9405a8703f6df655c17cdefc02f9977f92d3b2d8 and the moved around later. There is no problem with this commit. The actual segfault was introduced between the commits 9a9f08e089ff49fccca1feac9620c2837f8c09bd (good) and 64e30a7261e5a575a12beed1c3971f80779760f1 (bad). Overall diff of these is: https://github.com/numpy/numpy/compare/9a9f08e089ff49fccca1feac9620c2837f8c09bd...64e30a7261e5a575a12beed1c3971f80779760f1

The problem might be in the changes in the ctors.c file. At the moment, the correct fix (without leaks) is unclear. See the comments below for detailed analysis.

certik commented 12 years ago

Still fixes it:

diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c
index 618a871..4254d2f 100644
--- a/numpy/core/src/multiarray/calculation.c
+++ b/numpy/core/src/multiarray/calculation.c
@@ -959,12 +959,12 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
         indescr = PyArray_DESCR(self);
         Py_INCREF(indescr);
     }
-    Py_DECREF(newdescr);
+    //Py_DECREF(newdescr);

     if (!PyDataType_ISNOTSWAPPED(indescr)) {
         PyArray_Descr *descr2;
         descr2 = PyArray_DescrNewByteorder(indescr, '=');
-        Py_DECREF(indescr);
+        //Py_DECREF(indescr);
         if (descr2 == NULL) {
             goto fail;
         }
@@ -981,7 +981,7 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
     }
     else {
         /* Side-effect of PyArray_FromAny */
-        Py_DECREF(indescr);
+        //Py_DECREF(indescr);
     }

     /*
@@ -998,14 +998,14 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
             zero = PyInt_FromLong(0);
             cmp = PyObject_RichCompareBool(min, zero, Py_LT);
             if (cmp == -1) {
-                Py_DECREF(zero);
+                //Py_DECREF(zero);
                 goto fail;
             }
             if (cmp == 1) {
                 min = zero;
             }
             else {
-                Py_DECREF(zero);
+                //Py_DECREF(zero);
                 Py_INCREF(min);
             }
         }
@@ -1017,7 +1017,7 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
         Py_INCREF(indescr);
         mina = (PyArrayObject *)PyArray_FromAny(min, indescr, 0, 0,
                                  NPY_ARRAY_DEFAULT, NULL);
-        Py_DECREF(min);
+        //Py_DECREF(min);
         if (mina == NULL) {
             goto fail;
         }
@@ -1149,9 +1149,9 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
     /* Clean up temporary variables */
     Py_XDECREF(mina);
     Py_XDECREF(maxa);
-    Py_DECREF(newin);
+    //Py_DECREF(newin);
     /* Copy back into out if out was not already a nice array. */
-    Py_DECREF(newout);
+    //Py_DECREF(newout);
     return (PyObject *)out;

  fail:
certik commented 12 years ago

Still works:

diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c
index 618a871..d99b664 100644
--- a/numpy/core/src/multiarray/calculation.c
+++ b/numpy/core/src/multiarray/calculation.c
@@ -959,12 +959,12 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
         indescr = PyArray_DESCR(self);
         Py_INCREF(indescr);
     }
-    Py_DECREF(newdescr);
+    //Py_DECREF(newdescr);

     if (!PyDataType_ISNOTSWAPPED(indescr)) {
         PyArray_Descr *descr2;
         descr2 = PyArray_DescrNewByteorder(indescr, '=');
-        Py_DECREF(indescr);
+        //Py_DECREF(indescr);
         if (descr2 == NULL) {
             goto fail;
         }
@@ -981,7 +981,7 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
     }
     else {
         /* Side-effect of PyArray_FromAny */
-        Py_DECREF(indescr);
+        //Py_DECREF(indescr);
     }

     /*
@@ -998,14 +998,14 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
             zero = PyInt_FromLong(0);
             cmp = PyObject_RichCompareBool(min, zero, Py_LT);
             if (cmp == -1) {
-                Py_DECREF(zero);
+                //Py_DECREF(zero);
                 goto fail;
             }
             if (cmp == 1) {
                 min = zero;
             }
             else {
-                Py_DECREF(zero);
+                //Py_DECREF(zero);
                 Py_INCREF(min);
             }
         }
certik commented 12 years ago

Ok. Here is the fix:

diff --git a/numpy/core/src/multiarray/calculation.c b/numpy/core/src/multiarray/calculation.c
index 618a871..a360c4e 100644
--- a/numpy/core/src/multiarray/calculation.c
+++ b/numpy/core/src/multiarray/calculation.c
@@ -981,7 +981,7 @@ PyArray_Clip(PyArrayObject *self, PyObject *min, PyObject *max, PyArrayObject *o
     }
     else {
         /* Side-effect of PyArray_FromAny */
-        Py_DECREF(indescr);
+        //Py_DECREF(indescr);
     }

     /*

Now we need to figure out why this is needed, possibly due to some change in PyArray_FromAny() or something.

certik commented 12 years ago

So the Py_DECREF(indescr); code was originally added by 9405a8703f6df655c17cdefc02f9977f92d3b2d8 and then just moved around. Now the question is whether the bug was already present back then, or whether it was triggered by some of the code added later on.

certik commented 12 years ago

So the bug is not contained in the original commit. Bisection reveals that the bug was introduced somewhere in the commits:

4b157933641dc3b0241b9f1d68851a6661fa6604 ENH: core: Add static caching of the ca 64e30a7261e5a575a12beed1c3971f80779760f1 ENH: missingdata: Add skipna=, keepdims 6d9a2a1e7a13f076cea5e38194d56e045706c1ba ENH: missingdata: Add support for ident c9764c13eb0e1eef23345025c75577600b472ab3 ENH: missingdata: Future-proof AssignNA 6282b55d3262c134711e5ad4de62047c56866e8c ENH: missingdata: Move ReduceMaskNAArra 9ca27aecb17baee83a58d61507250d9aaa5ca34c ENH: missingdata: Add wheremask to PyAr c8c262a1be42c3989994bdc557c7b25e22d80d83 ENH: ufunc: Remove CreateReduceResult a 4b79354bd51f7702794fd8c0793adc82440f4c6f BUG: dtype: Mitigate crash bug for some d7076dcb8920e24d01507677efe4f206bd0fa1eb ENH: missingdata: Make ndarray.view val 0e1a4e9525b2c1e4abae97a6927cf59b5b2d534b ENH: missingdata: Add maskna= parameter 976476081b78279154950d2392aff8ee9290b60f DOC: Add info to the release notes abou 447d55d17136b8516a2ce49edae9ec82f0b00046 ENH: ufunc: Add a mask dtype parameter da2c9e4fa05b2df1062af519c7880286ab8d20c9 ENH: umath: Switch PyUFunc_Reduce to ca

But I am having problems testing these, because I am getting errors like:

(py27)ondrej@hawk:~/repos/numpy/numpy/core/src/multiarray((c8c262a...)|BISECTING)$ python a.py 
/home/ondrej/repos/numpy/py27/lib/python2.7/site-packages/numpy/random/__init__.py:87: RuntimeWarning: numpy.ndarray size changed, may indicate binary incompatibility
  from mtrand import *
OK 1
OK 2
Traceback (most recent call last):
  File "a.py", line 6, in <module>
    y = str(x)
  File "/home/ondrej/repos/numpy/py27/lib/python2.7/site-packages/numpy/core/numeric.py", line 1484, in array_str
    return array2string(a, max_line_width, precision, suppress_small, ' ', "", str)
  File "/home/ondrej/repos/numpy/py27/lib/python2.7/site-packages/numpy/core/arrayprint.py", line 459, in array2string
    separator, prefix, formatter=formatter)
  File "/home/ondrej/repos/numpy/py27/lib/python2.7/site-packages/numpy/core/arrayprint.py", line 260, in _array2string
    'float' : FloatFormat(data, precision, suppress_small),
  File "/home/ondrej/repos/numpy/py27/lib/python2.7/site-packages/numpy/core/arrayprint.py", line 546, in __init__
    self.fillFormat(data)
  File "/home/ondrej/repos/numpy/py27/lib/python2.7/site-packages/numpy/core/arrayprint.py", line 556, in fillFormat
    special = isnan(data) | isinf(data) | isna(data)
RuntimeError: field-NA is not supported yet

So I am still working on it. Here is my bisect log:

git bisect start
# good: [039ceba764704d0b4182cd2ecd6b88242f792804] REL: release Numpy 1.6.0
git bisect good 039ceba764704d0b4182cd2ecd6b88242f792804
# bad: [5c944b9c6915264380aa2df3dcfd0629867fbe80] Merge pull request #386 from certik/yop_atlas
git bisect bad 5c944b9c6915264380aa2df3dcfd0629867fbe80
# good: [8f860dcb7a258f23f806fed7889464a838b2e606] DOC: Fill in more of the nditer docs
git bisect good 8f860dcb7a258f23f806fed7889464a838b2e606
# bad: [07cf7f47f74986212a11ab0e13a187d3c2735a7b] FEAT: add wide unicode detection.
git bisect bad 07cf7f47f74986212a11ab0e13a187d3c2735a7b
# skip: [f308fcbbe2e6a0840804716822a4315ddefa45a2] ENH: umath: Add tests, work out kinks in ufunc 'where=' parameter
git bisect skip f308fcbbe2e6a0840804716822a4315ddefa45a2
# skip: [b48c7c1c7065a5fa12645473d6d288f2f7d81df1] DOC: Document the ufunc 'where=' parameter and the NpyAuxData C API mechanism
git bisect skip b48c7c1c7065a5fa12645473d6d288f2f7d81df1
# skip: [4b79354bd51f7702794fd8c0793adc82440f4c6f] BUG: dtype: Mitigate crash bug for some kinds of dtype inputs
git bisect skip 4b79354bd51f7702794fd8c0793adc82440f4c6f
# good: [9b354f4dc00e3aef4cfceae71be60b1dc60a1927] TST: Add test for ticket #1559.
git bisect good 9b354f4dc00e3aef4cfceae71be60b1dc60a1927
# good: [5f03b1504bcbe31b611376b6651e0297db165bad] ENH: core: Add support for masked strided transfer functions
git bisect good 5f03b1504bcbe31b611376b6651e0297db165bad
# good: [0c269ed28f73d373b5b0a9ba1e0aa6562ef360ed] ENH: ufunc: Split Reduce and Accumulate apart so that adding NA support is easier
git bisect good 0c269ed28f73d373b5b0a9ba1e0aa6562ef360ed
# good: [1848be621596d874b9e09afa1f7dd8175006980d] NEP: missingdata: Some fixes and updates to the NEP
git bisect good 1848be621596d874b9e09afa1f7dd8175006980d
# bad: [094992665c66ec66bb33042bfa1c1dc496e11b5c] ENH: core: Rename PyArrayObject_fieldaccess to PyArrayObject_fields
git bisect bad 094992665c66ec66bb33042bfa1c1dc496e11b5c
# bad: [4b157933641dc3b0241b9f1d68851a6661fa6604] ENH: core: Add static caching of the callables for C to core._method calls
git bisect bad 4b157933641dc3b0241b9f1d68851a6661fa6604
# good: [9a9f08e089ff49fccca1feac9620c2837f8c09bd] ENH: umath: Add checking for reorderable ufuncs, add PyArray_ReduceWrapper to API
git bisect good 9a9f08e089ff49fccca1feac9620c2837f8c09bd
# bad: [4b157933641dc3b0241b9f1d68851a6661fa6604] ENH: core: Add static caching of the callables for C to core._method calls
git bisect bad 4b157933641dc3b0241b9f1d68851a6661fa6604
# skip: [c8c262a1be42c3989994bdc557c7b25e22d80d83] ENH: ufunc: Remove CreateReduceResult and InitializeReduceResult from the API
git bisect skip c8c262a1be42c3989994bdc557c7b25e22d80d83
# skip: [c9764c13eb0e1eef23345025c75577600b472ab3] ENH: missingdata: Future-proof AssignNA and AssignMaskNA for later multi-NA support
git bisect skip c9764c13eb0e1eef23345025c75577600b472ab3
# skip: [9ca27aecb17baee83a58d61507250d9aaa5ca34c] ENH: missingdata: Add wheremask to PyArray_ContainsNA
git bisect skip 9ca27aecb17baee83a58d61507250d9aaa5ca34c
# skip: [976476081b78279154950d2392aff8ee9290b60f] DOC: Add info to the release notes about the full boolean indexing change
git bisect skip 976476081b78279154950d2392aff8ee9290b60f
certik commented 12 years ago

Also:

# skip: [d7076dcb8920e24d01507677efe4f206bd0fa1eb] ENH: missingdata: Make ndarray.view validate the maskna= parameter better
git bisect skip d7076dcb8920e24d01507677efe4f206bd0fa1eb
# skip: [6d9a2a1e7a13f076cea5e38194d56e045706c1ba] ENH: missingdata: Add support for identity-less skipna reductions with ndim > 1
git bisect skip 6d9a2a1e7a13f076cea5e38194d56e045706c1ba
# skip: [0e1a4e9525b2c1e4abae97a6927cf59b5b2d534b] ENH: missingdata: Add maskna= parameter to np.copy and ndarray.copy
git bisect skip 0e1a4e9525b2c1e4abae97a6927cf59b5b2d534b
# skip: [da2c9e4fa05b2df1062af519c7880286ab8d20c9] ENH: umath: Switch PyUFunc_Reduce to call PyArray_ReduceWrapper to simplify code
git bisect skip da2c9e4fa05b2df1062af519c7880286ab8d20c9
# bad: [4b157933641dc3b0241b9f1d68851a6661fa6604] ENH: core: Add static caching of the callables for C to core._method calls
git bisect bad 4b157933641dc3b0241b9f1d68851a6661fa6604
# skip: [6282b55d3262c134711e5ad4de62047c56866e8c] ENH: missingdata: Move ReduceMaskNAArray out of the public API
git bisect skip 6282b55d3262c134711e5ad4de62047c56866e8c
# skip: [447d55d17136b8516a2ce49edae9ec82f0b00046] ENH: ufunc: Add a mask dtype parameter to the masked ufunc loop selector
git bisect skip 447d55d17136b8516a2ce49edae9ec82f0b00046
# bad: [64e30a7261e5a575a12beed1c3971f80779760f1] ENH: missingdata: Add skipna=, keepdims= parameters to methods
git bisect bad 64e30a7261e5a575a12beed1c3971f80779760f1
certik commented 12 years ago

Ok, finally I am done:

There are only 'skip'ped commits left to test. The first bad commit could be any of: 4b79354bd51f7702794fd8c0793adc82440f4c6f c8c262a1be42c3989994bdc557c7b25e22d80d83 d7076dcb8920e24d01507677efe4f206bd0fa1eb 0e1a4e9525b2c1e4abae97a6927cf59b5b2d534b 9ca27aecb17baee83a58d61507250d9aaa5ca34c 6282b55d3262c134711e5ad4de62047c56866e8c 976476081b78279154950d2392aff8ee9290b60f 447d55d17136b8516a2ce49edae9ec82f0b00046 c9764c13eb0e1eef23345025c75577600b472ab3 6d9a2a1e7a13f076cea5e38194d56e045706c1ba da2c9e4fa05b2df1062af519c7880286ab8d20c9 64e30a7261e5a575a12beed1c3971f80779760f1 We cannot bisect more!

Alternate view:

* 64e30a7 (HEAD, refs/bisect/bad) ENH: missingdata: Add skipna=, keepdims= param
* 6d9a2a1 (refs/bisect/skip-6d9a2a1e7a13f076cea5e38194d56e045706c1ba) ENH: missi
* c9764c1 (refs/bisect/skip-c9764c13eb0e1eef23345025c75577600b472ab3) ENH: missi
* 6282b55 (refs/bisect/skip-6282b55d3262c134711e5ad4de62047c56866e8c) ENH: missi
* 9ca27ae (refs/bisect/skip-9ca27aecb17baee83a58d61507250d9aaa5ca34c) ENH: missi
* c8c262a (refs/bisect/skip-c8c262a1be42c3989994bdc557c7b25e22d80d83) ENH: ufunc
* 4b79354 (refs/bisect/skip-4b79354bd51f7702794fd8c0793adc82440f4c6f) BUG: dtype
* d7076dc (refs/bisect/skip-d7076dcb8920e24d01507677efe4f206bd0fa1eb) ENH: missi
* 0e1a4e9 (refs/bisect/skip-0e1a4e9525b2c1e4abae97a6927cf59b5b2d534b) ENH: missi
* 9764760 (refs/bisect/skip-976476081b78279154950d2392aff8ee9290b60f) DOC: Add i
* 447d55d (refs/bisect/skip-447d55d17136b8516a2ce49edae9ec82f0b00046) ENH: ufunc
* da2c9e4 (refs/bisect/skip-da2c9e4fa05b2df1062af519c7880286ab8d20c9) ENH: umath
* 9a9f08e (refs/bisect/good-9a9f08e089ff49fccca1feac9620c2837f8c09bd) ENH: umath

Unfortunately that's a lot of changes.

certik commented 12 years ago

So it might be coming from ctors.c. The changes that could broke it are (git diff 9a9f08e..64e30a7 ctors.c):

diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c
index 03c0356..ea1c940 100644
--- a/numpy/core/src/multiarray/ctors.c
+++ b/numpy/core/src/multiarray/ctors.c
@@ -1856,16 +1856,7 @@ PyArray_FromAny(PyObject *op, PyArray_Descr *newtype, int min_depth,
             ret = NULL;
         }
         else {
-            if (PyArray_HASMASKNA((PyArrayObject *)arr) &&
-                                (flags & NPY_ARRAY_ALLOWNA) == 0) {
-                PyErr_SetString(PyExc_ValueError,
-                        "this operation does not support "
-                        "arrays with NA masks");
-                ret = NULL;
-            }
-            else {
-                ret = (PyArrayObject *)PyArray_FromArray(arr, newtype, flags);
-            }
+            ret = (PyArrayObject *)PyArray_FromArray(arr, newtype, flags);
             Py_DECREF(arr);
         }
     }
@@ -1962,13 +1953,12 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
     int copy = 0;
     int arrflags;
     PyArray_Descr *oldtype;
-    PyTypeObject *subtype;
     NPY_CASTING casting = NPY_SAFE_CASTING;

     oldtype = PyArray_DESCR(arr);
-    subtype = Py_TYPE(arr);
     if (newtype == NULL) {
-        newtype = oldtype; Py_INCREF(oldtype);
+        newtype = oldtype;
+        Py_INCREF(oldtype);
     }
     itemsize = newtype->elsize;
     if (itemsize == 0) {
@@ -2005,141 +1995,79 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
         return NULL;
     }

-    /* Don't copy if sizes are compatible */
-    if ((flags & NPY_ARRAY_ENSURECOPY) ||
-                            PyArray_EquivTypes(oldtype, newtype)) {
-        arrflags = PyArray_FLAGS(arr);
-        if (PyArray_NDIM(arr) <= 1 && (flags & NPY_ARRAY_F_CONTIGUOUS)) {
-            flags |= NPY_ARRAY_C_CONTIGUOUS;
-        }
-        copy = (flags & NPY_ARRAY_ENSURECOPY) ||
-            ((flags & NPY_ARRAY_C_CONTIGUOUS) &&
-                    (!(arrflags & NPY_ARRAY_C_CONTIGUOUS)))
-            || ((flags & NPY_ARRAY_ALIGNED) &&
-                    (!(arrflags & NPY_ARRAY_ALIGNED)))
-            || (PyArray_NDIM(arr) > 1 &&
-                    ((flags & NPY_ARRAY_F_CONTIGUOUS) &&
-                    (!(arrflags & NPY_ARRAY_F_CONTIGUOUS))))
-            || ((flags & NPY_ARRAY_WRITEABLE) &&
-                    (!(arrflags & NPY_ARRAY_WRITEABLE)));
-
-        if (copy) {
-            if ((flags & NPY_ARRAY_UPDATEIFCOPY) &&
-                                (!PyArray_ISWRITEABLE(arr))) {
-                Py_DECREF(newtype);
-                PyErr_SetString(PyExc_ValueError,
-                        "cannot copy back to a read-only array");
-                return NULL;
-            }
-            if ((flags & NPY_ARRAY_ENSUREARRAY)) {
-                subtype = &PyArray_Type;
-            }
-            ret = (PyArrayObject *)
-                PyArray_NewFromDescr(subtype, newtype,
-                                     PyArray_NDIM(arr),
-                                     PyArray_DIMS(arr),
-                                     NULL, NULL,
-                                     flags & NPY_ARRAY_F_CONTIGUOUS,
-                                     (PyObject *)arr);
-            if (ret == NULL) {
-                return NULL;
-            }
-
-            /* Allocate an NA mask if necessary from the input */
-            if (PyArray_HASMASKNA(arr)) {
-                if (PyArray_AllocateMaskNA(ret, 1, 0, 1) < 0) {
-                    Py_DECREF(ret);
-                    return NULL;
-                }
-            }
-
-            if (PyArray_CopyInto(ret, arr) < 0) {
-                Py_DECREF(ret);
-                return NULL;
-            }
+    arrflags = PyArray_FLAGS(arr);
+    if (PyArray_NDIM(arr) <= 1 && (flags & NPY_ARRAY_F_CONTIGUOUS)) {
+        flags |= NPY_ARRAY_C_CONTIGUOUS;
+    }
+    copy = (flags & NPY_ARRAY_ENSURECOPY) ||
+        ((flags & NPY_ARRAY_C_CONTIGUOUS) &&
+                (!(arrflags & NPY_ARRAY_C_CONTIGUOUS)))
+        || ((flags & NPY_ARRAY_ALIGNED) &&
+                (!(arrflags & NPY_ARRAY_ALIGNED)))
+        || (PyArray_NDIM(arr) > 1 &&
+                ((flags & NPY_ARRAY_F_CONTIGUOUS) &&
+                (!(arrflags & NPY_ARRAY_F_CONTIGUOUS))))
+        || ((flags & NPY_ARRAY_WRITEABLE) &&
+                (!(arrflags & NPY_ARRAY_WRITEABLE))) ||
+           !PyArray_EquivTypes(oldtype, newtype);

-            /* Allocate an NA mask if requested but wasn't from the input */
-            if ((flags & (NPY_ARRAY_MASKNA | NPY_ARRAY_OWNMASKNA)) != 0 &&
-                                !PyArray_HASMASKNA(ret)) {
-                if (PyArray_AllocateMaskNA(ret, 1, 0, 1) < 0) {
-                    Py_DECREF(ret);
-                    return NULL;
-                }
-            }
+    if (copy) {
+        NPY_ORDER order = NPY_KEEPORDER;
+        int subok = 1;

-            if (flags & NPY_ARRAY_UPDATEIFCOPY)  {
-                /*
-                 * Don't use PyArray_SetBaseObject, because that compresses
-                 * the chain of bases.
-                 */
-                Py_INCREF(arr);
-                ((PyArrayObject_fieldaccess *)ret)->base = (PyObject *)arr;
-                PyArray_ENABLEFLAGS(ret, NPY_ARRAY_UPDATEIFCOPY);
-                PyArray_CLEARFLAGS(arr, NPY_ARRAY_WRITEABLE);
-            }
+        /* Set the order for the copy being made based on the flags */
+        if (flags & NPY_ARRAY_F_CONTIGUOUS) {
+            order = NPY_FORTRANORDER;
         }
-        /*
-         * If no copy then just increase the reference
-         * count and return the input
-         */
-        else {
-            Py_DECREF(newtype);
-            if ((flags & NPY_ARRAY_ENSUREARRAY) &&
-                                    !PyArray_CheckExact(arr)) {
-                PyArray_Descr *dtype = PyArray_DESCR(arr);
-                Py_INCREF(dtype);
-                ret = (PyArrayObject *)
-                    PyArray_NewFromDescr(&PyArray_Type,
-                                         dtype,
-                                         PyArray_NDIM(arr),
-                                         PyArray_DIMS(arr),
-                                         PyArray_STRIDES(arr),
-                                         PyArray_DATA(arr),
-                                         PyArray_FLAGS(arr),
-                                         NULL);
-                if (ret == NULL) {
-                    return NULL;
-                }
-                if (PyArray_SetBaseObject(ret, (PyObject *)arr)) {
-                    Py_DECREF(ret);
-                    return NULL;
-                }
-            }
-            else {
-                ret = arr;
-            }
-            Py_INCREF(arr);
+        else if (flags & NPY_ARRAY_C_CONTIGUOUS) {
+            order = NPY_CORDER;
         }
-    }

-    /*
-     * The desired output type is different than the input
-     * array type and copy was not specified
-     */
-    else {
         if ((flags & NPY_ARRAY_UPDATEIFCOPY) &&
                             (!PyArray_ISWRITEABLE(arr))) {
             Py_DECREF(newtype);
             PyErr_SetString(PyExc_ValueError,
-                    "cannot copy back to a read-only array B");
+                    "cannot copy back to a read-only array");
             return NULL;
         }
         if ((flags & NPY_ARRAY_ENSUREARRAY)) {
-            subtype = &PyArray_Type;
+            subok = 0;
         }
-        ret = (PyArrayObject *)
-            PyArray_NewFromDescr(subtype, newtype,
-                                 PyArray_NDIM(arr), PyArray_DIMS(arr),
-                                 NULL, NULL,
-                                 flags & NPY_ARRAY_F_CONTIGUOUS,
-                                 (PyObject *)arr);
+        ret = (PyArrayObject *)PyArray_NewLikeArray(arr, order,
+                                                    newtype, subok);
         if (ret == NULL) {
             return NULL;
         }
-        if (PyArray_CastTo(ret, arr) < 0) {
+
+        /*
+         * Allocate an NA mask if necessary from the input,
+         * is NAs are being allowed.
+         */
+        if (PyArray_HASMASKNA(arr) && (flags & NPY_ARRAY_ALLOWNA)) {
+            if (PyArray_AllocateMaskNA(ret, 1, 0, 1) < 0) {
+                Py_DECREF(ret);
+                return NULL;
+            }
+        }
+
+        /*
+         * If a ALLOWNA was not enabled, and 'arr' has an NA mask,
+         * this will raise an error if 'arr' contains any NA values.
+         */
+        if (PyArray_CopyInto(ret, arr) < 0) {
             Py_DECREF(ret);
             return NULL;
         }
+
+        /* Allocate an NA mask if requested but wasn't from the input */
+        if ((flags & (NPY_ARRAY_MASKNA | NPY_ARRAY_OWNMASKNA)) != 0 &&
+                            !PyArray_HASMASKNA(ret)) {
+            if (PyArray_AllocateMaskNA(ret, 1, 0, 1) < 0) {
+                Py_DECREF(ret);
+                return NULL;
+            }
+        }
+
         if (flags & NPY_ARRAY_UPDATEIFCOPY)  {
             /*
              * Don't use PyArray_SetBaseObject, because that compresses
@@ -2151,6 +2079,28 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
             PyArray_CLEARFLAGS(arr, NPY_ARRAY_WRITEABLE);
         }
     }
+    /*
+     * If no copy then just increase the reference
+     * count and return the input
+     */
+    else {
+        Py_DECREF(newtype);
+        if ((flags & NPY_ARRAY_ENSUREARRAY) &&
+                                !PyArray_CheckExact(arr)) {
+            PyArray_Descr *dtype = PyArray_DESCR(arr);
+            Py_INCREF(dtype);
+
+            ret = (PyArrayObject *)PyArray_View(arr, NULL, &PyArray_Type);
+            if (ret == NULL) {
+                return NULL;
+            }
+        }
+        else {
+            ret = arr;
+        }
+        Py_INCREF(arr);
+    }
+
     return (PyObject *)ret;
 }

@@ -2670,7 +2620,11 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
             baseflags |= NPY_ITER_USE_MASKNA;
         }
         else {
-            if (PyArray_ContainsNA(src)) {
+            int containsna = PyArray_ContainsNA(src, NULL, NULL);
+            if (containsna == -1) {
+                return -1;
+            }
+            else if (containsna) {
                 PyErr_SetString(PyExc_ValueError,
                         "Cannot assign NA to an array which "
                         "does not support NAs");
@@ -2684,7 +2638,7 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
      * to be exposed, then proceed without worrying about the mask.
      */
     else if (PyArray_HASMASKNA(dst)) {
-        if (PyArray_AssignMaskNA(dst, NULL, 1) < 0) {
+        if (PyArray_AssignMaskNA(dst, 1, NULL, 0, NULL) < 0) {
             return -1;
         }
         baseflags |= NPY_ITER_IGNORE_MASKNA;
charris commented 12 years ago

Just a heads up, C++ comments should not be used as some C compilers don't support them.

certik commented 12 years ago

(C++ comments: yes I know, I was just using this to quickly figure out where the problem is.)

certik commented 12 years ago

This is fixed by the PR #405, so I am closing this.