[arrow] branch master updated: ARROW-2029: [Python] NativeFile.tell errors after close

[we...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 68b119b  ARROW-2029: [Python] NativeFile.tell errors after close
68b119b is described below

commit 68b119b7c290f240c47cf54a2932bfd4794a10f8
Author: Jim Crist <ji...@gmail.com>
AuthorDate: Thu Jan 25 11:37:42 2018 -0500

    ARROW-2029: [Python] NativeFile.tell errors after close
    
    Previously checking if the file was closed was subclass specific, and wasn't caught in the hdfs backed file, leading to program crashes.
    
    This adds a check in `NativeFile.tell` for the file being open, and a test on a few subclasses of `NativeFile` to assure the error is raised.
    
    Note that since most python file-like objects raise a `ValueError` for operations after close, I changed the type of the existing error for these cases. This could be changed back, but an error should at least be thrown.
    
    Author: Jim Crist <ji...@gmail.com>
    
    Closes #1502 from jcrist/hdfs-tell-on-closed and squashes the following commits:
    
    8a9dc947 [Jim Crist] NativeFile.tell errors after close
---
 python/pyarrow/io.pxi           | 13 +++++++------
 python/pyarrow/tests/test_io.py | 26 +++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/python/pyarrow/io.pxi b/python/pyarrow/io.pxi
index 5449872..bb363ba 100644
--- a/python/pyarrow/io.pxi
+++ b/python/pyarrow/io.pxi
@@ -91,20 +91,20 @@ cdef class NativeFile:
         self._assert_writeable()
         file[0] = <shared_ptr[OutputStream]> self.wr_file
 
+    def _assert_open(self):
+        if not self.is_open:
+            raise ValueError("I/O operation on closed file")
+
     def _assert_readable(self):
+        self._assert_open()
         if not self.is_readable:
             raise IOError("only valid on readonly files")
 
-        if not self.is_open:
-            raise IOError("file not open")
-
     def _assert_writeable(self):
+        self._assert_open()
         if not self.is_writeable:
             raise IOError("only valid on writeable files")
 
-        if not self.is_open:
-            raise IOError("file not open")
-
     def size(self):
         """
         Return file size
@@ -120,6 +120,7 @@ cdef class NativeFile:
         Return current stream position
         """
         cdef int64_t position
+        self._assert_open()
         with nogil:
             if self.is_readable:
                 check_status(self.rd_file.get().Tell(&position))
diff --git a/python/pyarrow/tests/test_io.py b/python/pyarrow/tests/test_io.py
index e60dd35..3f7aa2e 100644
--- a/python/pyarrow/tests/test_io.py
+++ b/python/pyarrow/tests/test_io.py
@@ -257,7 +257,7 @@ def test_inmemory_write_after_closed():
     f.write(b'ok')
     f.get_result()
 
-    with pytest.raises(IOError):
+    with pytest.raises(ValueError):
         f.write(b'not ok')
 
 
@@ -503,3 +503,27 @@ def test_native_file_modes(tmpdir):
 
     with pa.memory_map(path, 'r+b') as f:
         assert f.mode == 'rb+'
+
+
+def test_native_file_raises_ValueError_after_close(tmpdir):
+    path = os.path.join(str(tmpdir), guid())
+    with open(path, 'wb') as f:
+        f.write(b'foooo')
+
+    with pa.OSFile(path, mode='rb') as os_file:
+        pass
+
+    with pa.memory_map(path, mode='rb') as mmap_file:
+        pass
+
+    files = [os_file,
+             mmap_file]
+
+    methods = [('tell', ()),
+               ('seek', (0,)),
+               ('size', ())]
+
+    for f in files:
+        for method, args in methods:
+            with pytest.raises(ValueError):
+                getattr(f, method)(*args)

-- 
To stop receiving notification emails like this one, please contact
wesm@apache.org.