Merge pull request #289 from matthew-brett/nasty-windows-fixes

MRG: nasty windows fixes

An absolutely horrible bug for MSVC casting from float to uint64 caused some
test failures on the buildbots.

Debugging, it did seem the code could work more robustly in a couple of cases.
Otherwise test for this horrible misfeature when present.

Some formatting changes and a fix for orderreddict testing when on Windows.

Delete temporary files to satisfy Windows' more stringent file attachments.

Author: matthew-brett

.travis.yml

@@ -27,8 +27,9 @@ matrix:
       env:
         - DEPENDS="numpy==1.5.1 pydicom==0.9.7"
 before_install:
-    - virtualenv venv
+    - virtualenv --python=python venv
     - source venv/bin/activate
+    - python --version # just to check
     - pip install nose # always
     - pip install --no-index -f http://travis-wheels.scikit-image.org $DEPENDS
     # pydicom <= 0.9.8 doesn't install on python 3

bin/parrec2nii

@@ -198,7 +198,7 @@ def proc_file(infile, opts):
     # container for potential NIfTI1 header extensions
     if opts.store_header:
         # dump the full PAR header content into an extension
-        with open(infile, 'r') as fobj:
+        with open(infile, 'rb') as fobj:  # contents must be bytes
             hdr_dump = fobj.read()
             dump_ext = nifti1.Nifti1Extension('comment', hdr_dump)
         nhdr.extensions.append(dump_ext)

nibabel/arraywriters.py

@@ -411,15 +411,16 @@ def _do_scaling(self):
     def _iu2iu(self):
         # (u)int to (u)int scaling
         mn, mx = self.finite_range()
-        if self._out_dtype.kind == 'u':
+        out_dt = self._out_dtype
+        if out_dt.kind == 'u':
             # We're checking for a sign flip.  This can only work for uint
             # output, because, for int output, the abs min of the type is
-            # greater than the abs max, so the data either fit into the range
-            # (tested for in _do_scaling), or this test can't pass
-            # Need abs that deals with max neg ints. abs problem only arises
-            # when all the data is set to max neg integer value
-            imax = np.iinfo(self._out_dtype).max
-            if mx <= 0 and int_abs(mn) <= imax: # sign flip enough?
+            # greater than the abs max, so the data either fits into the range
+            # (tested for in _do_scaling), or this test can't pass. Need abs
+            # that deals with max neg ints. abs problem only arises when all
+            # the data is set to max neg integer value
+            o_min, o_max = shared_range(self.scaler_dtype, out_dt)
+            if mx <= 0 and int_abs(mn) <= as_int(o_max): # sign flip enough?
                 # -1.0 * arr will be in scaler_dtype precision
                 self.slope = -1.0
                 return
@@ -563,15 +564,19 @@ def _iu2iu(self):
         # range may be greater than the largest integer for this type.
         # as_int needed to work round numpy 1.4.1 int casting bug
         out_dtype = self._out_dtype
-        t_min, t_max = np.iinfo(out_dtype).min, np.iinfo(out_dtype).max
-        type_range = as_int(t_max) - as_int(t_min)
+        # Options in this method are scaling using intercept only.  These will
+        # have to pass through ``self.scaler_dtype`` (because the intercept is
+        # in this type).
+        o_min, o_max = [as_int(v)
+                        for v in shared_range(self.scaler_dtype, out_dtype)]
+        type_range = o_max - o_min
         mn2mx = mx - mn
         if mn2mx <= type_range: # might offset be enough?
-            if t_min == 0: # uint output - take min to 0
+            if o_min == 0:  # uint output - take min to 0
                 # decrease offset with floor_exact, meaning mn >= t_min after
                 # subtraction.  But we may have pushed the data over t_max,
                 # which we check below
-                inter = floor_exact(mn - t_min, self.scaler_dtype)
+                inter = floor_exact(mn - o_min, self.scaler_dtype)
             else: # int output - take midpoint to 0
                 # ceil below increases inter, pushing scale up to 0.5 towards
                 # -inf, because ints have abs min == abs max + 1
@@ -581,15 +586,16 @@ def _iu2iu(self):
                 inter = floor_exact(midpoint, self.scaler_dtype)
             # Need to check still in range after floor_exact-ing
             int_inter = as_int(inter)
-            assert mn - int_inter >= t_min
-            if mx - int_inter <= t_max:
+            assert mn - int_inter >= o_min
+            if mx - int_inter <= o_max:
                 self.inter = inter
                 return
         # Try slope options (sign flip) and then range scaling
         super(SlopeInterArrayWriter, self)._iu2iu()
 
     def _range_scale(self, in_min, in_max):
-        """ Calculate scaling, intercept based on data range and output type """
+        """ Calculate scaling, intercept based on data range and output type
+        """
         if in_max == in_min: # Only one number in array
             self.slope = 1.
             self.inter = in_min
@@ -604,10 +610,10 @@ def _range_scale(self, in_min, in_max):
             in_min, in_max = np.array([in_min, in_max], dtype=big_float)
             in_range = np.diff([in_min, in_max])
         else: # max possible (u)int range is 2**64-1 (int64, uint64)
-            # int_to_float covers this range.  On windows longdouble is the same
-            # as double so in_range will be 2**64 - thus overestimating slope
-            # slightly.  Casting to int needed to allow in_max-in_min to be larger than
-            # the largest (u)int value
+            # int_to_float covers this range.  On windows longdouble is the
+            # same as double so in_range will be 2**64 - thus overestimating
+            # slope slightly.  Casting to int needed to allow in_max-in_min to
+            # be larger than the largest (u)int value
             in_min, in_max = as_int(in_min), as_int(in_max)
             in_range = int_to_float(in_max - in_min, big_float)
             # Cast to float for later processing.
@@ -624,13 +630,13 @@ def _range_scale(self, in_min, in_max):
             # raise an error when writing
             out_min, out_max = shared_range(working_dtype, out_dtype)
             out_min, out_max = np.array((out_min, out_max), dtype = big_float)
-        # We want maximum precision for the calculations. Casting will
-        # not lose precision because min/max are of fp type.
+        # We want maximum precision for the calculations. Casting will not lose
+        # precision because min/max are of fp type.
         assert [v.dtype.kind for v in (out_min, out_max)] == ['f', 'f']
         out_range = out_max - out_min
         """
-        Think of the input values as a line starting (left) at in_min and ending
-        (right) at in_max.
+        Think of the input values as a line starting (left) at in_min and
+        ending (right) at in_max.
 
         The output values will be a line starting at out_min and ending at
         out_max.
@@ -666,20 +672,20 @@ def _range_scale(self, in_min, in_max):
         We can't change the range of the saved data (the whole range of the
         integer type) or the range of the output data (the values we input). We
         can change the intermediate values ``saved_data * slope`` by choosing
-        the sign of the slope to match the in_min or in_max to the left or right
-        end of the saved data range.
+        the sign of the slope to match the in_min or in_max to the left or
+        right end of the saved data range.
 
-        If the out_dtype is signed int, then abs(out_min) = abs(out_max) + 1 and
-        the absolute value and therefore precision for values at the left and
-        right of the saved data range are very similar (e.g. -128 * slope, 127 *
-        slope respectively).
+        If the out_dtype is signed int, then abs(out_min) = abs(out_max) + 1
+        and the absolute value and therefore precision for values at the left
+        and right of the saved data range are very similar (e.g. -128 * slope,
+        127 * slope respectively).
 
-        If the out_dtype is unsigned int, then the absolute value at the left is
-        0 and the precision is much higher than for the right end of the range
-        (e.g. 0 * slope, 255 * slope).
+        If the out_dtype is unsigned int, then the absolute value at the left
+        is 0 and the precision is much higher than for the right end of the
+        range (e.g. 0 * slope, 255 * slope).
 
-        If the out_dtype is unsigned int then we choose the sign of the slope to
-        match the smaller of the in_min, in_max to the zero end of the saved
+        If the out_dtype is unsigned int then we choose the sign of the slope
+        to match the smaller of the in_min, in_max to the zero end of the saved
         range.
         """
         if out_min == 0 and np.abs(in_max) < np.abs(in_min):

nibabel/casting.py

@@ -14,6 +14,14 @@ class CastingError(Exception):
     pass
 
 
+# Test for VC truncation when casting floats to uint64
+# Christoph Gohlke says this is so for MSVC <= 2010 because VC is using x87
+# instructions; see:
+# https://github.com/scipy/scipy/blob/99bb8411f6391d921cb3f4e56619291e91ddf43b/scipy/ndimage/tests/test_datatypes.py#L51
+_test_val = 2**63 + 2**11 # Should be exactly representable in float64
+TRUNC_UINT64 = np.float64(_test_val).astype(np.uint64) != _test_val
+
+
 def float_to_int(arr, int_type, nan2zero=True, infmax=False):
     """ Convert floating point array `arr` to type `int_type`
 
@@ -107,8 +115,8 @@ def shared_range(flt_type, int_type):
     """ Min and max in float type that are >=min, <=max in integer type
 
     This is not as easy as it sounds, because the float type may not be able to
-    exactly represent the max or min integer values, so we have to find the next
-    exactly representable floating point value to do the thresholding.
+    exactly represent the max or min integer values, so we have to find the
+    next exactly representable floating point value to do the thresholding.
 
     Parameters
     ----------
@@ -151,6 +159,8 @@ def shared_range(flt_type, int_type):
     mx = floor_exact(ii.max, flt_type)
     if mx == np.inf:
         mx = fi.max
+    elif TRUNC_UINT64 and int_type == np.uint64:
+        mx = min(mx, flt_type(2**63))
     _SHARED_RANGES[key] = (mn, mx)
     return mn, mx
 

nibabel/externals/tests/test_ordereddict.py

@@ -12,9 +12,9 @@
 
 def test_right_version():
     # If Python < 2.7, use our own copy, else use system copy
-    class_path = inspect.getfile(OrderedDict)
-    rel_dir = dirname(relpath(realpath(class_path), MY_PATH))
     if sys.version_info[:2] < (2, 7):
+        class_path = realpath(inspect.getfile(OrderedDict))
+        rel_dir = dirname(relpath(class_path, MY_PATH))
         assert_equal(rel_dir, '..')
     else:
         import collections

nibabel/tests/test_arraywriters.py

@@ -135,17 +135,40 @@ def test_no_scaling():
         kwargs = (dict(check_scaling=False) if awt == ArrayWriter
                   else dict(calc_scale=False))
         aw = awt(arr, out_dtype, **kwargs)
-        with suppress_warnings():  # cast to real from cplx
+        with suppress_warnings():
             back_arr = round_trip(aw)
-            exp_back = arr.astype(float)
+        exp_back = arr.copy()
+        # If converting to floating point type, casting is direct.
+        # Otherwise we will need to do float-(u)int casting at some point.
         if out_dtype in IUINT_TYPES:
-            with np.errstate(invalid='ignore'):
-                exp_back = np.round(exp_back)
-            if hasattr(aw, 'slope') and in_dtype in FLOAT_TYPES:
-                # Finite scaling sets infs to min / max
-                exp_back = np.clip(exp_back, 0, 1)
-            else:
-                exp_back = np.clip(exp_back, *shared_range(float, out_dtype))
+            if in_dtype in CFLOAT_TYPES:
+                # Working precision is (at least) float
+                with suppress_warnings():
+                    exp_back = exp_back.astype(float)
+                # Float to iu conversion will always round, clip
+                with np.errstate(invalid='ignore'):
+                    exp_back = np.round(exp_back)
+                if hasattr(aw, 'slope') and in_dtype in FLOAT_TYPES:
+                    # Finite scaling sets infs to min / max
+                    exp_back = np.clip(exp_back, 0, 1)
+                else:
+                    # Clip to shared range of working precision
+                    exp_back = np.clip(exp_back,
+                                       *shared_range(float, out_dtype))
+            else:  # iu input and output type
+                # No scaling, never gets converted to float.
+                # Does get clipped to range of output type
+                mn_out, mx_out = _dt_min_max(out_dtype)
+                if (mn_in, mx_in) != (mn_out, mx_out):
+                    # Use smaller of input, output range to avoid np.clip
+                    # upcasting the array because of large clip limits.
+                    exp_back = np.clip(exp_back,
+                                       max(mn_in, mn_out),
+                                       min(mx_in, mx_out))
+        elif in_dtype in COMPLEX_TYPES:
+            # always cast to real from complex
+            with suppress_warnings():
+                exp_back = exp_back.astype(float)
         exp_back = exp_back.astype(out_dtype)
         # Sometimes working precision is float32 - allow for small differences
         assert_allclose_safely(back_arr, exp_back)

nibabel/tests/test_nifti1.py

@@ -627,7 +627,7 @@ def test_float128(self):
             assert_raises(HeaderDataError, hdr.set_data_dtype, np.longdouble)
 
 
-class TestNifti1Pair(tana.TestAnalyzeImage, tspm.ScalingMixin):
+class TestNifti1Pair(tana.TestAnalyzeImage, tspm.ImageScalingMixin):
     # Run analyze-flavor spatialimage tests
     image_class = Nifti1Pair
     supported_np_types = TestNifti1PairHeader.supported_np_types

nibabel/tests/test_scripts.py

@@ -96,22 +96,29 @@ def check_conversion(cmd, pr_data, out_fname):
     assert_true(np.allclose(data, pr_data))
     assert_true(np.allclose(img.header['cal_min'], data.min()))
     assert_true(np.allclose(img.header['cal_max'], data.max()))
+    del img, data  # for windows to be able to later delete the file
     # Check minmax options
     run_command(cmd + ['--minmax', '1', '2'])
     img = load(out_fname)
+    data = img.get_data()
     assert_true(np.allclose(data, pr_data))
     assert_true(np.allclose(img.header['cal_min'], 1))
     assert_true(np.allclose(img.header['cal_max'], 2))
+    del img, data  # for windows
     run_command(cmd + ['--minmax', 'parse', '2'])
     img = load(out_fname)
+    data = img.get_data()
     assert_true(np.allclose(data, pr_data))
     assert_true(np.allclose(img.header['cal_min'], data.min()))
     assert_true(np.allclose(img.header['cal_max'], 2))
+    del img, data  # for windows
     run_command(cmd + ['--minmax', '1', 'parse'])
     img = load(out_fname)
+    data = img.get_data()
     assert_true(np.allclose(data, pr_data))
     assert_true(np.allclose(img.header['cal_min'], 1))
     assert_true(np.allclose(img.header['cal_max'], data.max()))
+    del img, data
 
 
 @script_test
@@ -134,6 +141,8 @@ def test_parrec2nii():
             assert_almost_equal(img.header.get_zooms(), eg_dict['zooms'])
             # Standard save does not save extensions
             assert_equal(len(img.header.extensions), 0)
+            # Delete previous img, data to make Windows happier
+            del img, data
             # Does not overwrite unless option given
             code, stdout, stderr = run_command(
                 ['parrec2nii', fname], check_code=False)
@@ -161,6 +170,7 @@ def test_parrec2nii():
             run_command(base_cmd + ['--store-header'])
             img = load(out_froot)
             assert_equal(len(img.header.extensions), 1)
+            del img  # To help windows delete the file
 
 
 @script_test