From e5e0cc963130842a3bba9c6929332e8f15c08372 Mon Sep 17 00:00:00 2001
From: Paul McCarthy <pauld.mccarthy@gmail.com>
Date: Wed, 15 Jun 2016 10:48:08 +0100
Subject: [PATCH] Bugfix in ImageWrapper.__expandCoverage - variable name
 clashes.

Bugfix in ImageWrapper.calcExpansion - could not handle slice outside of
coverage, but overlapping with some dimensions.

Initial support in Image class for data editing.
---
 fsl/data/image.py        | 58 +++++++++++++++++++++++++++++++++------
 fsl/data/imagewrapper.py | 59 +++++++++++++++++++++++++++-------------
 2 files changed, 90 insertions(+), 27 deletions(-)

diff --git a/fsl/data/image.py b/fsl/data/image.py
index b624d6b80..802172675 100644
--- a/fsl/data/image.py
+++ b/fsl/data/image.py
@@ -458,13 +458,14 @@ class Image(Nifti1, notifier.Notifier):
  
         Nifti1.__init__(self, nibImage.get_header())
 
-        self.name           = name
-        self.__dataSource   = dataSource
-        self.__nibImage     = nibImage
-        self.__saveState    = dataSource is not None
-        self.__imageWrapper = imagewrapper.ImageWrapper(self.nibImage,
-                                                        self.name,
-                                                        loadData=loadData)
+        self.name                = name
+        self.__dataSource        = dataSource
+        self.__nibImage          = nibImage
+        self.__saveState         = dataSource is not None
+        self.__suppressDataRange = False
+        self.__imageWrapper      = imagewrapper.ImageWrapper(self.nibImage,
+                                                             self.name,
+                                                             loadData=loadData)
 
         if calcRange:
             self.calcRange()
@@ -561,7 +562,8 @@ class Image(Nifti1, notifier.Notifier):
         Notifies any listeners of this ``Image`` (registered through the
         :class:`.Notifier` interface) on the ``'dataRange'`` topic.
         """
-        self.notify(notifier_topic='dataRange')
+        if not self.__suppressDataRange:
+            self.notify(notifier_topic='dataRange')
 
 
     def calcRange(self, sizethres=None):
@@ -606,11 +608,19 @@ class Image(Nifti1, notifier.Notifier):
         self.__imageWrapper.loadData()
 
 
+    def save(self, filename=None):
+        raise NotImplementedError()
+
+
     def __getitem__(self, sliceobj):
         """Access the image data with the specified ``sliceobj``.
 
         :arg sliceobj: Something which can slice the image data.
         """
+
+
+        log.debug('{}: __getitem__ [{}]'.format(self.name, sliceobj))
+        
         data = self.__imageWrapper.__getitem__(sliceobj)
 
         if len(data.shape) > len(self.shape):
@@ -621,6 +631,38 @@ class Image(Nifti1, notifier.Notifier):
         return data
 
 
+    def __setitem__(self, sliceobj, values):
+        """Set the image data at ``sliceobj`` to ``values``.
+
+        :arg sliceobj: Something which can slice the image data.
+        :arg values:   New image data.
+
+        .. note:: Modifying image data will force the entire image to be 
+                  loaded into memory if it has not already been loaded.
+        """
+
+        log.debug('{}: __setitem__ [{} = {}]'.format(self.name,
+                                                     sliceobj,
+                                                     values.shape))
+
+        self.__suppressDataRange = True
+        oldRange = self.__imageWrapper.dataRange
+
+        self.__imageWrapper.__setitem__(sliceobj, values)
+
+        newRange = self.__imageWrapper.dataRange
+        self.__suppressDataRange = False
+
+        if values.size > 0:
+            
+            self.__saveState = False
+            self.notify(notifier_topic='data')
+            self.notify(notifier_topic='saveState')
+
+            if not np.all(np.isclose(oldRange, newRange)):
+                self.notify(notifier_topic='dataRange') 
+
+
 # TODO The wx.FileDialog does not    
 # seem to handle wildcards with      
 # multiple suffixes (e.g. '.nii.gz'),
diff --git a/fsl/data/imagewrapper.py b/fsl/data/imagewrapper.py
index 0ae4fb36d..7d0f21ea5 100644
--- a/fsl/data/imagewrapper.py
+++ b/fsl/data/imagewrapper.py
@@ -336,19 +336,19 @@ class ImageWrapper(notifier.Notifier):
         given ``slices``.
         """
 
-        log.debug('Updating image {} data range [slice: {}] (current range: '
-                  '[{}, {}]; current coverage: {})'.format(
+        volumes, expansions = calcExpansion(slices, self.__coverage)
+
+        log.debug('Updating image {} data range [slice: {}] '
+                  '(current range: [{}, {}]; '
+                  'number of expansions: {}; '
+                  'current coverage: {})'.format(
                       self.__name,
                       slices,
                       self.__range[0],
                       self.__range[1],
+                      len(volumes),
                       self.__coverage))
         
-        volumes, expansions = calcExpansion(slices, self.__coverage)
-        
-        oldmin, oldmax = self.__range
-        newmin, newmax = oldmin, oldmax
-
         # The calcExpansion function splits up the
         # expansions on volumes - here we calculate
         # the min/max per volume/expansion, and
@@ -356,39 +356,41 @@ class ImageWrapper(notifier.Notifier):
         # coverage and data range.
         for vol, exp in zip(volumes, expansions):
 
-            oldmin, oldmax = self.__volRanges[vol, :]
+            oldvmin, oldvmax = self.__volRanges[vol, :]
 
-            data   = self.__getData(exp, isTuple=True)
-            newmin = float(np.nanmin(data))
-            newmax = float(np.nanmax(data))
+            data    = self.__getData(exp, isTuple=True)
+            newvmin = float(np.nanmin(data))
+            newvmax = float(np.nanmax(data))
 
-            if (not np.isnan(oldmin)) and oldmin < newmin: newmin = oldmin
-            if (not np.isnan(oldmax)) and oldmax > newmax: newmax = oldmax
+            if (not np.isnan(oldvmin)) and oldvmin < newvmin: newvmin = oldvmin
+            if (not np.isnan(oldvmax)) and oldvmax > newvmax: newvmax = oldvmax
 
             # Update the stored range and
             # coverage for each volume 
-            self.__volRanges[vol, :]  = newmin, newmax
+            self.__volRanges[vol, :]  = newvmin, newvmax
             self.__coverage[..., vol] = adjustCoverage(
                 self.__coverage[..., vol], exp)
 
         # Calculate the new known data
         # range over the entire image
         # (i.e. over all volumes).
-        newmin = np.nanmin(self.__volRanges[:, 0])
-        newmax = np.nanmax(self.__volRanges[:, 1])
+        newmin = float(np.nanmin(self.__volRanges[:, 0]))
+        newmax = float(np.nanmax(self.__volRanges[:, 1]))
 
+        oldmin, oldmax = self.__range
         self.__range   = (newmin, newmax)
         self.__covered = self.__imageIsCovered()
 
-        if not np.all(np.isclose([oldmin, newmin], [oldmax, newmax])):
+        if any((oldmin is None, oldmax is None)) or \
+           not np.all(np.isclose([oldmin, oldmax], [newmin, newmax])):
             log.debug('Image {} range changed: [{}, {}] -> [{}, {}]'.format(
                 self.__name,
                 oldmin,
                 oldmax,
                 newmin,
                 newmax))
-            self.notify() 
-    
+            self.notify()
+
 
     def __updateDataRangeOnRead(self, slices, data):
         """Called by :meth:`__getitem__`. Calculates the minimum/maximum
@@ -446,6 +448,13 @@ class ImageWrapper(notifier.Notifier):
             #      coverage is necessary?
             lowVol, highVol = slices[self.__numRealDims - 1]
 
+            log.debug('Image {} data written - clearing known data '
+                      'range on volumes {} - {} (write slice: {})'.format(
+                          self.__name,
+                          lowVol,
+                          highVol,
+                          slices))
+
             for vol in range(lowVol, highVol):
                 self.__coverage[:, :, vol]    = np.nan
                 self.__volRanges[     vol, :] = np.nan
@@ -800,6 +809,18 @@ def calcExpansion(slices, coverage):
                 expansion[dimy][0] = expLow
                 expansion[dimy][1] = expHigh
 
+
+            # If no range exists for any of the
+            # other dimensions, the range for
+            # all expansions will be the current
+            # coverage
+            for dimy in range(numDims):
+                if dimy == dimx:
+                    continue
+
+                if np.any(np.isnan(expansion[dimy])):
+                    expansion[dimy] = coverage[:, dimy, vol]
+
             # Finish off this expansion
             expansion = finishExpansion(expansion, vol)
 
-- 
GitLab