diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 15d563eeb0021411574dd267935b0310c1aedfee..d4920d3747b9171938e5fe2a1a53d246770e28d8 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -2,6 +2,18 @@ This document contains the ``fslpy`` release history in reverse chronological
order.
+1.10.2 (Friday September 7th 2018)
+----------------------------------
+
+
+Fixed
+^^^^^
+
+
+* The :meth:`.Image.save` method was not handling memory-mapped images
+ correctly.
+
+
1.10.1 (Friday August 3rd 2018)
-------------------------------
@@ -16,7 +28,7 @@ Changed
Fixed
^^^^^
-* The .mod:`.FEATImage.getCOPE` method was returning PE images.
+* The :mod:`.FEATImage.getCOPE` method was returning PE images.
1.10.0 (Wednesday July 18th 2018)
diff --git a/fsl/data/image.py b/fsl/data/image.py
index 0a130dcaad1169a847f5900fb27ac8f7081ee065..7af1e696053f3c94648f66810134127b9120a11f 100644
--- a/fsl/data/image.py
+++ b/fsl/data/image.py
@@ -35,8 +35,10 @@ and file names:
import os
import os.path as op
+import shutil
import string
import logging
+import tempfile
import six
import deprecation
@@ -1165,11 +1167,41 @@ class Image(Nifti):
log.debug('Saving {} to {}'.format(self.name, filename))
# If this Image is not managing its
- # own file object, nibabel does all
- # of the hard work.
- if self.__fileobj is None:
+ # own file object, and the image is not
+ # memory-mapped, nibabel does all of
+ # the hard work.
+ newnibimage = False
+ ismmap = isinstance(self[0, 0, :10], np.memmap)
+ if self.__fileobj is None and (not ismmap):
nib.save(self.__nibImage, filename)
+ # If the image is memory-mapped, we need
+ # to close and re-open the image
+ elif self.__fileobj is None:
+
+ # We save the image out to a temp file,
+ # then close the old image, move the
+ # temp file to the real destination,
+ # then re-open the file.
+ newnibimage = True
+ tmphd, tmpfname = tempfile.mkstemp(suffix=op.splitext(filename)[1])
+ os.close(tmphd)
+
+ try:
+ nib.save(self.__nibImage, tmpfname)
+
+ self.__nibImage = None
+ self.header = None
+
+ shutil.copy(tmpfname, filename)
+
+ self.__nibImage = nib.load(filename)
+ self.header = self.__nibImage.header
+
+ except Exception:
+ os.remove(tmpfname)
+ raise
+
# Otherwise we've got our own file
# handle to an IndexedGzipFile
else:
@@ -1190,15 +1222,19 @@ class Image(Nifti):
# compressed data. And then be able to
# transfer the index generated from the
# write to a new read-only file handle.
+ newnibimage = True
nib.save(self.__nibImage, filename)
self.__fileobj.close()
self.__nibImage, self.__fileobj = loadIndexedImageFile(filename)
self.header = self.__nibImage.header
- # We have to create a new ImageWrapper
- # instance too, as we have just destroyed
- # the nibabel image we gave to the last
- # one.
+ # If we've created a new nibabel image,
+ # we have to create a new ImageWrapper
+ # instance too, as we have just destroyed
+ # the nibabel image we gave to the last
+ # one.
+ if newnibimage:
+
self.__imageWrapper.deregister(self.__lName)
self.__imageWrapper = imagewrapper.ImageWrapper(
self.nibImage,
diff --git a/fsl/data/imagewrapper.py b/fsl/data/imagewrapper.py
index d2b4fb9169a862dbb102807a06abba44036bb546..559c0d974f5563edbafef459870cbbb5b504ddac 100644
--- a/fsl/data/imagewrapper.py
+++ b/fsl/data/imagewrapper.py
@@ -825,6 +825,8 @@ def expectedShape(sliceobj, shape):
if start is None: start = 0
if stop is None: stop = shape[i]
+ stop = min(stop, shape[i])
+
expShape.append(stop - start)
return len(expShape), expShape
diff --git a/tests/test_image.py b/tests/test_image.py
index da6263912057ddd902624a89c6999d2bd805760c..84a3d244542163e7c164a26a0408c336447af250 100644
--- a/tests/test_image.py
+++ b/tests/test_image.py
@@ -994,29 +994,29 @@ def _test_Image_save(imgtype):
make_image(filename, imgtype)
- # Using mmap can cause a "Bus error"
- # under docker. No idea why.
- img = fslimage.Image(filename, mmap=False)
+ # Save to original location, and
+ # to a different location. And
+ # test both with and without mmap
+ targets = [None, filename, filename2]
+ mmaps = [False, True]
- randvoxes = randvoxes(5)
- randvals = [np.random.random() for i in range(5)]
+ for target, mmap in it.product(targets, mmaps):
- for (x, y, z), v in zip(randvoxes, randvals):
- img[x, y, z] = v
+ img = fslimage.Image(filename, mmap=mmap)
- if imgtype > 0:
- img.voxToWorldMat = xform
+ rvoxs = randvoxes(5)
+ rvals = [np.random.random() for i in range(5)]
- # Save to original location, and
- # to a different location
- targets = [None, filename, filename2]
+ for (x, y, z), v in zip(rvoxs, rvals):
+ img[x, y, z] = v
- for t in targets:
+ if imgtype > 0:
+ img.voxToWorldMat = xform
- img.save(t)
+ img.save(target)
- if t is None: expDataSource = filename
- else: expDataSource = t
+ if target is None: expDataSource = filename
+ else: expDataSource = target
assert img.saveState
assert img.dataSource == expDataSource
@@ -1024,7 +1024,7 @@ def _test_Image_save(imgtype):
if imgtype > 0:
assert np.all(np.isclose(img.voxToWorldMat, xform))
- for (x, y, z), v in zip(randvoxes, randvals):
+ for (x, y, z), v in zip(rvoxs, rvals):
assert np.isclose(img[x, y, z], v)
# Load the image back in
@@ -1036,7 +1036,7 @@ def _test_Image_save(imgtype):
if imgtype > 0:
assert np.all(np.isclose(img.voxToWorldMat, xform))
- for (x, y, z), v in zip(randvoxes, randvals):
+ for (x, y, z), v in zip(rvoxs, rvals):
assert np.isclose(img[x, y, z], v)
img2 = None
diff --git a/tests/test_imagewrapper.py b/tests/test_imagewrapper.py
index b074358d28f6ee05101fa4e7a9a714249c7820dc..1237cd55d02966fc5596c32825dcec071f9c8b2e 100644
--- a/tests/test_imagewrapper.py
+++ b/tests/test_imagewrapper.py
@@ -257,6 +257,55 @@ def coverageDataRange(data, coverage, slices=None):
return np.min(volmin), np.max(volmax)
+def test_expectedShape():
+
+ tests = [
+ ((slice(None), ), (10,),
+ (1, (10, ))),
+
+ ((slice(None), slice(None)),
+ (10, 10), (2, (10, 10))),
+
+ ((slice(None), slice(None), slice(None)),
+ (10, 10, 10), (3, (10, 10, 10))),
+
+ ((slice(None), slice(None), slice(None)),
+ (10, 10, 10), (3, (10, 10, 10))),
+
+ ((slice(None), slice(None), slice(None), slice(None)),
+ (10, 10, 10, 10), (4, (10, 10, 10, 10))),
+
+ ((1, slice(None), slice(None)),
+ (10, 10, 10), (2, (10, 10))),
+
+ ((slice(1, 3), slice(None), slice(None)),
+ (10, 10, 10), (3, (2, 10, 10))),
+
+ ((slice(None), 1, slice(None)),
+ (10, 10, 10), (2, (10, 10))),
+
+ ((slice(None), slice(1, 3), slice(None)),
+ (10, 10, 10), (3, (10, 2, 10))),
+
+ ((slice(None), slice(None), 1),
+ (10, 10, 10), (2, (10, 10))),
+
+ ((slice(None), slice(None), slice(1, 3), ),
+ (10, 10, 10), (3, (10, 10, 2))),
+
+ ((slice(None), slice(None), slice(1, 20), ),
+ (10, 10, 10), (3, (10, 10, 9))),
+ ]
+
+ for slc, shape, exp in tests:
+
+ explen, exp = exp
+ gotlen, got = imagewrap.expectedShape(slc, shape)
+
+ assert explen == gotlen
+ assert tuple(exp) == tuple(got)
+
+
def test_sliceObjToSliceTuple():
func = imagewrap.sliceObjToSliceTuple