Bugfix/workaround - due to the non-guaranteed ordering of property listener...

Bugfix/workaround - due to the non-guaranteed ordering of property listener callbacks, not all SliceCanvas objecst were updating their colour textures on changes to range clipping. You can now add a 'preNotifyFunc' to PropertyBase objects, which will be called as soon as a property value changes, before any listeners. A few other minor commentings and refactorings

fsl/data/fslimage.py

 #!/usr/bin/env python
 #
-# fslimage.py - Object representing a 3D image.
+# fslimage.py - Classes representing a 3D image, and the display
+# properties of a 3D image.
 #
 # Author: Paul McCarthy <pauldmccarthy@gmail.com>
 #
@@ -13,9 +14,17 @@ import matplotlib.colors  as mplcolors
 import fsl.props          as props
 import fsl.data.imagefile as imagefile
 
+
 class Image(object):
+    """
+    Class which represents a 3D image. Interally, the image is loaded/stored
+    using nibabel.
+    """
 
     def __init__(self, image):
+        """
+        Initialise an Image object with the given image data or file name.
+        """
 
         # The image parameter may be the name of an image file
         if isinstance(image, str):
@@ -43,15 +52,30 @@ class Image(object):
         self.zlen  = zlen
 
 
-        
 class ImageDisplay(props.HasProperties):
     """
+    A class which describes how an image should be displayed.
     """
 
+
+    def updateColourMap(self, newVal):
+        """
+        When a colour property changes, this method is called -
+        it reconfigures the colour map accordingly.
+        """
+
+        if self.rangeClip:
+            self.cmap.set_under(self.cmap(0.0), alpha=0.0)
+            self.cmap.set_over( self.cmap(1.0), alpha=0.0)
+        else:
+            self.cmap.set_under(self.cmap(0.0), alpha=1.0)
+            self.cmap.set_over( self.cmap(1.0), alpha=1.0)   
+
     alpha      = props.Double(minval=0.0, maxval=1.0, default=1.0)
     displayMin = props.Double()
     displayMax = props.Double()
-    rangeClip  = props.Boolean(default=False)
+    rangeClip  = props.Boolean(default=False,
+                               preNotifyFunc=updateColourMap)
 
     _view   = props.VGroup(('displayMin', 'displayMax', 'alpha', 'rangeClip'))
     _labels = {
@@ -63,6 +87,10 @@ class ImageDisplay(props.HasProperties):
 
 
     def __init__(self, image):
+        """
+        Create an ImageDisplay for the specified image. The image
+        parameter should be an Image object (defined above).
+        """
 
         self.image = image
 
@@ -73,18 +101,3 @@ class ImageDisplay(props.HasProperties):
         self.dataMax    = self.image.data.max() 
         self.displayMin = self.dataMin    # use cal_min/cal_max instead?
         self.displayMax = self.dataMax
-
-        self.addListener(
-            'rangeClip',
-            'ImageRangeClip_{}'.format(id(self)),
-            self.updateColourMap)
-
-    def updateColourMap(self, *a):
-
-        if self.rangeClip:
-            self.cmap.set_under(self.cmap(0.0), alpha=0.0)
-            self.cmap.set_over( self.cmap(1.0), alpha=0.0)
-        else:
-            self.cmap.set_under(self.cmap(0.0), alpha=1.0)
-            self.cmap.set_over( self.cmap(1.0), alpha=1.0)   
-

fsl/props/properties.py

@@ -128,6 +128,8 @@
 import types
 import logging
 
+from collections import OrderedDict
+
 log = logging.getLogger(__name__)
 
 
@@ -149,7 +151,6 @@ class PropertyValue(object):
           - value: Initial value.
           - name:  Variable name - if not provided, a default,
                    unique name is created.
-
         """
         
         if name is None: name = '{}_{}'.format(prop.label, id(self))
@@ -157,7 +158,7 @@ class PropertyValue(object):
         self.prop            = prop
         self.owner           = owner
         self.name            = name
-        self.changeListeners = {}
+        self.changeListeners = OrderedDict()
         self._value          = value
         self._lastValue      = value
         self._lastValid      = self.isValid()
@@ -219,6 +220,11 @@ class PropertyValue(object):
 
         log.debug('Variable {} changed: {}'.format(self.name, newValue))
 
+        # Call the PropertyBase prenotify function first, if there is one
+        if self.prop.preNotifyFunc is not None:
+            log.debug('Calling preNotify function for {}'.format(self.name))
+            self.prop.preNotifyFunc(self.owner, newValue)
+
         # Validate the new value and notify any registered listeners
         self.validateAndNotify()
             
@@ -302,30 +308,40 @@ class PropertyBase(object):
 
     """
     
-    def __init__(self, default=None, required=False, validateFunc=None):
+    def __init__(self,
+                 default=None,
+                 required=False,
+                 validateFunc=None,
+                 preNotifyFunc=None):
         """
         Parameters:
         
-          - default:      Default/initial value.
+          - default:       Default/initial value.
         
-          - required:     Boolean determining whether or not this
-                          property must have a value. May alternately
-                          be a function which accepts one parameter,
-                          the owning HasProperties instance, and
-                          returns True or False.
+          - required:      Boolean determining whether or not this
+                           property must have a value. May alternately
+                           be a function which accepts one parameter,
+                           the owning HasProperties instance, and
+                           returns True or False.
         
-          - validateFunc: Custom validation function. Must accept
-                          two parameters: a reference to the
-                          HasProperties instance, the owner of
-                          this property; and the new property
-                          value. Should return True if the property
-                          value is valid, False otherwise.
+          - validateFunc:  Custom validation function. Must accept
+                           two parameters: a reference to the
+                           HasProperties instance, the owner of
+                           this property; and the new property
+                           value. Should return True if the property
+                           value is valid, False otherwise.
+
+          - preNotifyFunc: Function to be called whenever the property
+                           value(s) changes. This function is called
+                           by the PropertyValue object(s) before any
+                           listeners are notified.
         """
         self.label             = None
         self.default           = default
         self.required          = required
         self.validateFunc      = validateFunc
-        self.changeListeners   = {}
+        self.preNotifyFunc     = preNotifyFunc
+        self.changeListeners   = OrderedDict()
 
         
     def addListener(self, instance, name, callback):

fsl/utils/slicecanvas.py

@@ -197,7 +197,9 @@ class SliceCanvas(wxgl.GLCanvas):
         
           parent - WX parent object
         
-          image  - A fsl.data.image.Image object, or a 3D numpy array.
+          image  - A fsl.data.fslimage.Image object, or a
+                   fsl.data.fslimage.ImageDisplay object, or a 3D numpy
+                   array.
         
           zax    - Axis perpendicular to the plane to be displayed
                    (the 'depth' axis), default 0.
@@ -272,34 +274,34 @@ class SliceCanvas(wxgl.GLCanvas):
 
         self.Bind(wx.EVT_PAINT, self.draw)
 
+        # Add a bunch of listeners to the image display
+        # object, so we can update the view when image
+        # display properties are changed.
         def refreshNeeded(*a):
             self.Refresh()
             
         def colourUpdateNeeded(*a):
             self.updateColourBuffer()
-            self.Refresh() 
+            self.Refresh()
+
+        lnrName = 'SliceCanvas_{{}}_{}'.format(id(self))
 
         self.imageDisplay.addListener(
-            'alpha',
-            'SliceCanvasAlpha_{}'.format(id(self)),
-            refreshNeeded)
+            'alpha', lnrName.format('alpha'), refreshNeeded)
+        
         self.imageDisplay.addListener(
-            'displayMin',
-            'SliceCanvasDisplayMin_{}'.format(id(self)),
-            colourUpdateNeeded)
+            'displayMin', lnrName.format('displayMin'), colourUpdateNeeded)
+        
         self.imageDisplay.addListener(
-            'displayMax',
-            'SliceCanvasDisplayMax_{}'.format(id(self)),
-            colourUpdateNeeded)
+            'displayMax', lnrName.format('displayMax'), colourUpdateNeeded)
+        
         self.imageDisplay.addListener(
-            'rangeClip',
-            'SliceCanvasRangeClip_{}'.format(id(self)),
-            colourUpdateNeeded) 
+            'rangeClip', lnrName.format('rangeClip'), colourUpdateNeeded) 
 
 
     def _initGLData(self):
         """
-        Initialises the GL buffers which are  copied to the GPU,
+        Initialises the GL buffers which are copied to the GPU,
         and used to render the voxel data.
         """
 
@@ -319,7 +321,7 @@ class SliceCanvas(wxgl.GLCanvas):
             shaders.compileShader(vertex_shader,   gl.GL_VERTEX_SHADER),
             shaders.compileShader(fragment_shader, gl.GL_FRAGMENT_SHADER))
 
-        # Indexes of all vertex/fragment shader parameters 
+        # Indices of all vertex/fragment shader parameters 
         self.inVertexPos   = gl.glGetAttribLocation( self.shaders, 'inVertex')
         self.voxelValuePos = gl.glGetAttribLocation( self.shaders, 'voxelValue')
         self.inPositionPos = gl.glGetAttribLocation( self.shaders, 'inPosition')
@@ -391,16 +393,25 @@ class SliceCanvas(wxgl.GLCanvas):
 
         iDisplay = self.imageDisplay
 
+        # Here we are creating a range of values to be passed
+        # to the matplotlib.colors.Colormap instance of the
+        # image display. We scale this range such that data
+        # values which lie outside the configured display range
+        # will map to values below 0.0 or above 1.0. It is
+        # assumed that the Colormap instance is configured to
+        # generate appropriate colours for these out-of-range
+        # values.
+        
+        normalRange = np.linspace(0.0, 1.0, self.colourResolution)
+        normalStep  = 1.0 / (self.colourResolution - 1) 
+
         normMin = (iDisplay.displayMin - iDisplay.dataMin) / \
                   (iDisplay.dataMax    - iDisplay.dataMin)
         normMax = (iDisplay.displayMax - iDisplay.dataMin) / \
                   (iDisplay.dataMax    - iDisplay.dataMin)
 
-        normalStep  = 1.0 / (self.colourResolution - 1)
-        newStep     = normalStep / (normMax - normMin)
-
-        normalRange = np.linspace(0.0, 1.0, self.colourResolution)
-        newRange    = (normalRange - normMin) * (newStep / normalStep)
+        newStep  = normalStep / (normMax - normMin)
+        newRange = (normalRange - normMin) * (newStep / normalStep)
 
         # Create [self.colourResolution] rgb values,
         # spanning the entire range of the image