Merge pull request #6632 from mcmtroffaes/feature/fromfile-ioopen-bug

TST/BUG: fromfile - fix test and expose bug with io class argument

6 files changed, 45 insertions, 36 deletions

diff --git a/numpy/core/include/numpy/npy_3kcompat.h b/numpy/core/include/numpy/npy_3kcompat.h
index cdab1bbe8..c0aa1eb2e 100644
--- a/numpy/core/include/numpy/npy_3kcompat.h
+++ b/numpy/core/include/numpy/npy_3kcompat.h
@@ -148,7 +148,7 @@ PyUnicode_Concat2(PyObject **left, PyObject *right)
 /*
  * PyFile_* compatibility
  */
-#if defined(NPY_PY3K)
+
 /*
  * Get a FILE* handle to the file represented by the Python object
  */
@@ -160,6 +160,13 @@ npy_PyFile_Dup2(PyObject *file, char *mode, npy_off_t *orig_pos)
     npy_off_t pos;
     FILE *handle;
 
+    /* For Python 2 PyFileObject, use PyFile_AsFile */
+#if !defined(NPY_PY3K)
+    if (PyFile_Check(file)) {
+        return PyFile_AsFile(file);
+    }
+#endif
+
     /* Flush first to ensure things end up in the file in the correct order */
     ret = PyObject_CallMethod(file, "flush", "");
     if (ret == NULL) {
@@ -257,6 +264,13 @@ npy_PyFile_DupClose2(PyObject *file, FILE* handle, npy_off_t orig_pos)
     PyObject *ret, *io, *io_raw;
     npy_off_t position;
 
+    /* For Python 2 PyFileObject, do nothing */
+#if !defined(NPY_PY3K)
+    if (PyFile_Check(file)) {
+        return 0;
+    }
+#endif
+
     position = npy_ftell(handle);
 
     /* Close the FILE* handle */
@@ -314,6 +328,12 @@ static NPY_INLINE int
 npy_PyFile_Check(PyObject *file)
 {
     int fd;
+    /* For Python 2, check if it is a PyFileObject */
+#if !defined(NPY_PY3K)
+    if (PyFile_Check(file)) {
+        return 1;
+    }
+#endif
     fd = PyObject_AsFileDescriptor(file);
     if (fd == -1) {
         PyErr_Clear();
@@ -322,32 +342,6 @@ npy_PyFile_Check(PyObject *file)
     return 1;
 }
 
-#else
-
-static NPY_INLINE FILE *
-npy_PyFile_Dup2(PyObject *file,
-                const char *NPY_UNUSED(mode), npy_off_t *NPY_UNUSED(orig_pos))
-{
-    FILE * fp = PyFile_AsFile(file);
-    if (fp == NULL) {
-        PyErr_SetString(PyExc_IOError,
-                        "first argument must be an open file");
-        return NULL;
-    }
-    return fp;
-}
-
-static NPY_INLINE int
-npy_PyFile_DupClose2(PyObject *NPY_UNUSED(file), FILE* NPY_UNUSED(handle),
-                     npy_off_t NPY_UNUSED(orig_pos))
-{
-    return 0;
-}
-
-#define npy_PyFile_Check PyFile_Check
-
-#endif
-
 static NPY_INLINE PyObject*
 npy_PyFile_OpenFile(PyObject *filename, const char *mode)
 {
diff --git a/numpy/core/records.py b/numpy/core/records.py
index 7ad0c111a..ecc293812 100644
--- a/numpy/core/records.py
+++ b/numpy/core/records.py
@@ -723,7 +723,8 @@ def fromfile(fd, dtype=None, shape=None, offset=0, formats=None,
     """Create an array from binary file data
 
     If file is a string then that file is opened, else it is assumed
-    to be a file object.
+    to be a file object. The file object must support random access
+    (i.e. it must have tell and seek methods).
 
     >>> from tempfile import TemporaryFile
     >>> a = np.empty(10,dtype='f8,i4,a5')
diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c
index c30b38109..945ae9804 100644
--- a/numpy/core/src/multiarray/methods.c
+++ b/numpy/core/src/multiarray/methods.c
@@ -563,7 +563,7 @@ array_tofile(PyArrayObject *self, PyObject *args, PyObject *kwds)
     FILE *fd;
     char *sep = "";
     char *format = "";
-    npy_off_t orig_pos;
+    npy_off_t orig_pos = 0;
     static char *kwlist[] = {"file", "sep", "format", NULL};
 
     if (!PyArg_ParseTupleAndKeywords(args, kwds, "O|ss:tofile", kwlist,
diff --git a/numpy/core/src/multiarray/multiarraymodule.c b/numpy/core/src/multiarray/multiarraymodule.c
index babbdf581..2fdabb187 100644
--- a/numpy/core/src/multiarray/multiarraymodule.c
+++ b/numpy/core/src/multiarray/multiarraymodule.c
@@ -2067,7 +2067,7 @@ array_fromfile(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *keywds)
     static char *kwlist[] = {"file", "dtype", "count", "sep", NULL};
     PyArray_Descr *type = NULL;
     int own;
-    npy_off_t orig_pos;
+    npy_off_t orig_pos = 0;
     FILE *fp;
 
     if (!PyArg_ParseTupleAndKeywords(args, keywds,
diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py
index df201f582..213ae7a52 100644
--- a/numpy/core/tests/test_multiarray.py
+++ b/numpy/core/tests/test_multiarray.py
@@ -3903,7 +3903,7 @@ class TestLexsort(TestCase):
         x = np.linspace(0., 1., 42*3).reshape(42, 3)
         assert_raises(np.AxisError, np.lexsort, x, axis=2)
 
-class TestIO(object):
+class TestIO(TestCase):
     """Test tofile, fromfile, tobytes, and fromstring"""
 
     def setUp(self):
@@ -3994,17 +3994,23 @@ class TestIO(object):
         y = np.fromstring(s, sep="@")
         assert_array_equal(x, y)
 
-    def test_unbuffered_fromfile(self):
+    def test_unseekable_fromfile(self):
         # gh-6246
         self.x.tofile(self.filename)
 
         def fail(*args, **kwargs):
-            raise io.IOError('Can not tell or seek')
+            raise IOError('Can not tell or seek')
 
         with io.open(self.filename, 'rb', buffering=0) as f:
             f.seek = fail
             f.tell = fail
-            y = np.fromfile(self.filename, dtype=self.dtype)
+            self.assertRaises(IOError, np.fromfile, f, dtype=self.dtype)
+
+    def test_io_open_unbuffered_fromfile(self):
+        # gh-6632
+        self.x.tofile(self.filename)
+        with io.open(self.filename, 'rb', buffering=0) as f:
+            y = np.fromfile(f, dtype=self.dtype)
             assert_array_equal(y, self.x.flat)
 
     def test_largish_file(self):
@@ -4028,6 +4034,13 @@ class TestIO(object):
         assert_equal(os.path.getsize(self.filename), d.nbytes * 2)
 
 
+    def test_io_open_buffered_fromfile(self):
+        # gh-6632
+        self.x.tofile(self.filename)
+        f = io.open(self.filename, 'rb', buffering=-1)
+        y = np.fromfile(f, dtype=self.dtype)
+        assert_array_equal(y, self.x.flat)
+
     def test_file_position_after_fromfile(self):
         # gh-4118
         sizes = [io.DEFAULT_BUFFER_SIZE//8,
diff --git a/numpy/lib/tests/test_io.py b/numpy/lib/tests/test_io.py
index 2a494fe08..868089551 100644
--- a/numpy/lib/tests/test_io.py
+++ b/numpy/lib/tests/test_io.py
@@ -103,8 +103,9 @@ class RoundtripTest(object):
             if not isinstance(target_file, BytesIO):
                 target_file.close()
                 # holds an open file descriptor so it can't be deleted on win
-                if not isinstance(arr_reloaded, np.lib.npyio.NpzFile):
-                    os.remove(target_file.name)
+                if 'arr_reloaded' in locals():
+                    if not isinstance(arr_reloaded, np.lib.npyio.NpzFile):
+                        os.remove(target_file.name)
 
     def check_roundtrips(self, a):
         self.roundtrip(a)