Merge pull request #383 from bcipolli/issue-362

MRG: Fix for issue 362: Python 3.5 fails reading large gzipped files 

This not a nibabel bug, but works around a Python bug (https://bugs.python.org/issue25626).

The workaround is: we wrap gzip.GzipFile with buffering, so that files > 4GB require multiple calls to GzipFile.readinto.

I've also added test functionality: if NIPY_EXTRA_TESTS contains 'slow', slowly running tests can be run. I've used this to include a test for creation of a large file.

Author: matthew-brett

doc/source/devel/advanced_testing.rst

@@ -0,0 +1,32 @@
+.. -*- mode: rst -*-
+.. ex: set sts=4 ts=4 sw=4 et tw=79:
+  ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ###
+  #
+  #   See COPYING file distributed along with the NiBabel package for the
+  #   copyright and license terms.
+  #
+  ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ###
+
+.. _advanced_testing:
+
+************
+Advanced Testing
+************
+
+Setup
+-----
+
+Before running advanced tests, please update all submodules of nibabel, by running ``git submodule update --init``
+
+
+Long-running tests
+------------------
+
+Long-running tests are not enabled by default, and can be resource-intensive. To run these tests:
+
+* Set environment variable ``NIPY_EXTRA_TESTS=slow``
+* Run ``nosetests``.
+
+Note that some tests may require a machine with >4GB of RAM.
+
+.. include:: ../links_names.txt

doc/source/devel/index.rst

@@ -14,3 +14,4 @@ Developer documentation page
     add_image_format
     devdiscuss
     make_release
+    advanced_testing

doc/source/installation.rst

@@ -114,7 +114,11 @@ Just install the modules by invoking::
 If sudo is not configured (or even installed) you might have to use
 ``su`` instead.
 
-Now fire up Python and try importing the module to see if everything is fine.
+
+Validating your install
+-----------------------
+
+For a basic test of your installation, fire up Python and try importing the module to see if everything is fine.
 It should look something like this::
 
     Python 2.7.8 (v2.7.8:ee879c0ffa11, Jun 29 2014, 21:07:35)
@@ -123,4 +127,9 @@ It should look something like this::
     >>> import nibabel
     >>>
 
+
+To run the nibabel test suite, from the terminal run ``nosetests nibabel`` or ``python -c "import nibabel; nibabel.test()``.
+
+To run an extended test suite that validates ``nibabel`` for long-running and resource-intensive cases, please see :ref:`advanced_testing`.
+
 .. include:: links_names.txt

nibabel/openers.py

@@ -9,19 +9,65 @@
 """ Context manager openers for various fileobject types
 """
 
-from os.path import splitext
-import gzip
 import bz2
+import gzip
+import sys
+from os.path import splitext
+
 
 # The largest memory chunk that gzip can use for reads
 GZIP_MAX_READ_CHUNK = 100 * 1024 * 1024  # 100Mb
 
 
+class BufferedGzipFile(gzip.GzipFile):
+    """GzipFile able to readinto buffer >= 2**32 bytes.
+
+    This class only differs from gzip.GzipFile
+    in Python 3.5.0.
+
+    This works around a known issue in Python 3.5.
+    See https://bugs.python.org/issue25626"""
+
+    # This helps avoid defining readinto in Python 2.6,
+    #   where it is undefined on gzip.GzipFile.
+    # It also helps limit the exposure to this code.
+    if sys.version_info[:3] == (3, 5, 0):
+        def __init__(self, fileish, mode='rb', compresslevel=9,
+                     buffer_size=2**32-1):
+            super(BufferedGzipFile, self).__init__(fileish, mode=mode,
+                                                   compresslevel=compresslevel)
+            self.buffer_size = buffer_size
+
+        def readinto(self, buf):
+            """Uses self.buffer_size to do a buffered read."""
+            n_bytes = len(buf)
+            if n_bytes < 2 ** 32:
+                return super(BufferedGzipFile, self).readinto(buf)
+
+            # This works around a known issue in Python 3.5.
+            # See https://bugs.python.org/issue25626
+            mv = memoryview(buf)
+            n_read = 0
+            max_read = 2 ** 32 - 1  # Max for unsigned 32-bit integer
+            while (n_read < n_bytes):
+                n_wanted = min(n_bytes - n_read, max_read)
+                n_got = super(BufferedGzipFile, self).readinto(
+                    mv[n_read:n_read + n_wanted])
+                n_read += n_got
+                if n_got != n_wanted:
+                    break
+            return n_read
+
+
 def _gzip_open(fileish, *args, **kwargs):
-    # open gzip files with faster reads on large files using larger chunks
+    gzip_file = BufferedGzipFile(fileish, *args, **kwargs)
+
+    # Speedup for #209; attribute not present in in Python 3.5
+    # open gzip files with faster reads on large files using larger
     # See https://github.com/nipy/nibabel/pull/210 for discussion
-    gzip_file = gzip.open(fileish, *args, **kwargs)
-    gzip_file.max_read_chunk = GZIP_MAX_READ_CHUNK
+    if hasattr(gzip_file, 'max_chunk_read'):
+        gzip_file.max_read_chunk = GZIP_MAX_READ_CHUNK
+
     return gzip_file
 
 

nibabel/testing/__init__.py

@@ -9,12 +9,14 @@
 ''' Utilities for testing '''
 from __future__ import division, print_function
 
+import os
 import sys
 import warnings
 from os.path import dirname, abspath, join as pjoin
 
 import numpy as np
 
+from numpy.testing.decorators import skipif
 # Allow failed import of nose if not now running tests
 try:
     from nose.tools import (assert_equal, assert_not_equal,
@@ -164,3 +166,12 @@ def __init__(self, *args, **kwargs):
         warnings.warn('catch_warn_reset is deprecated and will be removed in '
                       'nibabel v3.0; use nibabel.testing.clear_and_catch_warnings.',
                       FutureWarning)
+
+
+EXTRA_SET = os.environ.get('NIPY_EXTRA_TESTS', '').split(',')
+
+
+def runif_extra_has(test_str):
+    """Decorator checks to see if NIPY_EXTRA_TESTS env var contains test_str"""
+    return skipif(test_str not in EXTRA_SET,
+                  "Skip {0} tests.".format(test_str))

nibabel/tests/test_nifti1.py

@@ -13,17 +13,18 @@
 
 import numpy as np
 
-from ..externals.six import BytesIO
-from ..casting import type_info, have_binary128
-from ..tmpdirs import InTemporaryDirectory
-from ..spatialimages import HeaderDataError
-from ..eulerangles import euler2mat
-from ..affines import from_matvec
-from .. import nifti1 as nifti1
-from ..nifti1 import (load, Nifti1Header, Nifti1PairHeader, Nifti1Image,
-                      Nifti1Pair, Nifti1Extension, Nifti1Extensions,
-                      data_type_codes, extension_codes, slice_order_codes)
-
+from nibabel import nifti1 as nifti1
+from nibabel.affines import from_matvec
+from nibabel.casting import type_info, have_binary128
+from nibabel.eulerangles import euler2mat
+from nibabel.externals.six import BytesIO
+from nibabel.nifti1 import (load, Nifti1Header, Nifti1PairHeader, Nifti1Image,
+                            Nifti1Pair, Nifti1Extension, Nifti1Extensions,
+                            data_type_codes, extension_codes,
+                            slice_order_codes)
+from nibabel.openers import ImageOpener
+from nibabel.spatialimages import HeaderDataError
+from nibabel.tmpdirs import InTemporaryDirectory
 from ..freesurfer import load as mghload
 
 from .test_arraywriters import rt_err_estimate, IUINT_TYPES
@@ -35,7 +36,7 @@
 from nose.tools import (assert_true, assert_false, assert_equal,
                         assert_raises)
 
-from ..testing import data_path, suppress_warnings
+from ..testing import data_path, suppress_warnings, runif_extra_has
 
 from . import test_analyze as tana
 from . import test_spm99analyze as tspm
@@ -1242,3 +1243,19 @@ def test_rt_bias(self):
                 # Hokey use of max_miss as a std estimate
                 bias_thresh = np.max([max_miss / np.sqrt(count), eps])
                 assert_true(np.abs(bias) < bias_thresh)
+
+
+@runif_extra_has('slow')
+def test_large_nifti1():
+    image_shape = (91, 109, 91, 1200)
+    img = Nifti1Image(np.ones(image_shape, dtype=np.float32),
+                      affine=np.eye(4))
+    # Dump and load the large image.
+    with InTemporaryDirectory():
+        img.to_filename('test.nii.gz')
+        del img
+        data = load('test.nii.gz').get_data()
+    # Check that the data are all ones
+    assert_equal(image_shape, data.shape)
+    n_ones = np.sum((data == 1.))
+    assert_equal(np.prod(image_shape), n_ones)