Merge branch 'rf/numpy_array_writeable' into 'master'

Rf/numpy array writeable

See merge request fsl/fslpy!116

CHANGELOG.rst

@@ -12,6 +12,20 @@ Added
 
 * New tensor conversion routines in the :mod:`.dtifit` module (Michiel
   Cottaar).
+* New :func:`.makeWriteable` function which ensures that a ``numpy.array`` is
+  writeable, and creates a copy if necessary
+
+
+Changed
+^^^^^^^
+
+
+* The :class:`.GiftiMesh` class no longer creates copies of the mesh
+  vertex/index arrays. This means that, with ``numpy>=1.16`` these arrays
+  will be flagged as read-only.
+* The :class:`.Mesh` class handles vertex data sets requiring different
+  triangle unwinding orders, at the cost of potentially having to store
+  two copies of the mesh indices.
 
 
 Fixed

fsl/data/__init__.py

@@ -9,4 +9,5 @@ models, constants, and other data-like things used throughout ``fslpy``.
 """
 
 
-from .utils import guessType  # noqa
+from .utils import (guessType,  # noqa
+                    makeWriteable)

fsl/data/gifti.py

@@ -211,8 +211,8 @@ def loadGiftiMesh(filename):
         raise ValueError('{}: GIFTI surface files must contain '
                          'at least one pointset array'.format(filename))
 
-    vertices = [np.array(ps.data) for ps in pointsets]
-    indices  = np.array(triangles[0].data)
+    vertices = [ps.data for ps in pointsets]
+    indices  = triangles[0].data
 
     if len(vdata) == 0: vdata = None
     else:               vdata = prepareGiftiVertexData(vdata, filename)

fsl/data/mesh.py

@@ -98,6 +98,12 @@ class Mesh(notifier.Notifier, meta.Meta):
        selectedVertices
        vertexSets
 
+    .. note:: Internally the ``Mesh`` class may store two versions of the
+              triangles, with opposite unwinding orders. It keeps track of the
+              required triangle unwinding order for each vertex set, so that
+              the :meth:`indices` method will return the appropriate copy for
+              the currently selected vertex set.
+
 
     **Vertex data**
 
@@ -183,19 +189,17 @@ class Mesh(notifier.Notifier, meta.Meta):
 
         self.__name       = name
         self.__dataSource = dataSource
-        self.__indices    = np.asarray(indices).reshape((-1, 3))
-        self.__nvertices  = self.__indices.max() + 1
-
-        # This attribute is used to store
-        # the currently selected vertex set,
-        # used as a kety into all of the
-        # dictionaries below.
-        self.__selected = None
-
-        # Flag used to keep track of whether
-        # the triangle winding order has been
-        # "fixed" - see the addVertices method.
-        self.__fixed = False
+        self.__nvertices  = indices.max() + 1
+        self.__selected   = None
+
+        # We potentially store two copies of
+        # the indices, with opposite unwinding
+        # orders. The vindices dict stores refs
+        # to one or the other for each vertex
+        # set.
+        self.__indices      = np.asarray(indices).reshape((-1, 3))
+        self.__fixedIndices = None
+        self.__vindices     = collections.OrderedDict()
 
         # All of these are populated
         # in the addVertices method
@@ -287,7 +291,7 @@ class Mesh(notifier.Notifier, meta.Meta):
     @property
     def indices(self):
         """The ``(M, 3)`` triangles of this mesh. """
-        return self.__indices
+        return self.__vindices[self.__selected]
 
 
     @property
@@ -297,7 +301,7 @@ class Mesh(notifier.Notifier, meta.Meta):
         """
 
         selected = self.__selected
-        indices  = self.__indices
+        indices  = self.__vindices[selected]
         vertices = self.__vertices[selected]
         fnormals = self.__faceNormals.get(selected, None)
 
@@ -315,7 +319,7 @@ class Mesh(notifier.Notifier, meta.Meta):
         """
 
         selected = self.__selected
-        indices  = self.__indices
+        indices  = self.__vindices[selected]
         vertices = self.__vertices[selected]
         vnormals = self.__vertNormals.get(selected, None)
 
@@ -334,10 +338,8 @@ class Mesh(notifier.Notifier, meta.Meta):
 
             ``((xlow, ylow, zlow), (xhigh, yhigh, zhigh))``
         """
-
         lo = self.__loBounds[self.__selected]
         hi = self.__hiBounds[self.__selected]
-
         return lo, hi
 
 
@@ -411,26 +413,26 @@ class Mesh(notifier.Notifier, meta.Meta):
                                                     self.nvertices))
 
         self.__vertices[key] = vertices
+        self.__vindices[key] = self.__indices
         self.__loBounds[key] = lo
         self.__hiBounds[key] = hi
 
         if select:
             self.vertices = key
 
-        # indices already fixed?
-        if fixWinding and (not self.__fixed):
-            indices      = self.indices
-            normals      = self.normals
-            needsFix     = needsFixing(vertices, indices, normals, lo, hi)
-            self.__fixed = True
+        if fixWinding:
+            indices  = self.__indices
+            normals  = self.normals
+            needsFix = needsFixing(vertices, indices, normals, lo, hi)
 
             # See needsFixing documentation
             if needsFix:
 
-                indices[:, [1, 2]] = indices[:, [2, 1]]
+                if self.__fixedIndices is None:
+                    self.__fixedIndices = indices[:, [0, 2, 1]]
 
-                for k, fn in self.__faceNormals.items():
-                    self.__faceNormals[k] = fn * -1
+                self.__vindices[   key] = self.__fixedIndices
+                self.__faceNormals[key] = normals * -1
 
         return vertices
 

fsl/data/utils.py

@@ -10,7 +10,7 @@ with the data types defined in the :mod:`fsl.data` package.
 
 
 import os.path as op
-
+import numpy   as np
 
 def guessType(path):
     """A convenience function which, given the name of a file or directory,
@@ -77,3 +77,14 @@ def guessType(path):
     # Otherwise, I don't
     # know what to do
     return None, path
+
+
+def makeWriteable(array):
+    """Updates the given ``numpy.array`` so that it is writeable. If this
+    is not possible, a copy is created and returned.
+    """
+    try:
+        array.flags['WRITEABLE'] = True
+    except ValueError:
+        array = np.array(array)
+    return array

tests/test_fsl_data_utils.py

@@ -9,6 +9,8 @@ import            shutil
 import            os
 import os.path as op
 
+import numpy   as np
+
 import fsl.utils.tempdir        as tempdir
 import fsl.data.utils           as dutils
 
@@ -105,3 +107,22 @@ def test_guessType():
         asrt('norecognise.txt', None)
         os.remove('norecognise')
         os.remove('norecognise.txt')
+
+
+def test_makeWriteable():
+    robuf = bytes(    b'\01\02\03\04')
+    wbuf  = bytearray(b'\01\02\03\04')
+
+    roarr = np.ndarray((4,), dtype=np.uint8, buffer=robuf)
+    warr  = np.ndarray((4,), dtype=np.uint8, buffer=wbuf)
+
+    warr.flags['WRITEABLE'] = False
+
+    rocopy = dutils.makeWriteable(roarr)
+    wcopy  = dutils.makeWriteable(warr)
+
+    assert rocopy.base is not roarr.base
+    assert wcopy .base is     warr .base
+
+    rocopy[1] = 100
+    wcopy[ 1] = 100

tests/test_mesh.py

@@ -417,3 +417,29 @@ def test_planeIntersection():
     assert lines.shape  == (0, 2, 3)
     assert faces.shape  == (0, )
     assert dists.shape  == (0, 2, 3)
+
+
+def test_mesh_different_winding_orders():
+
+    verts1    =  CUBE_VERTICES
+    verts2    = -CUBE_VERTICES
+    tris      =  CUBE_TRIANGLES_CCW
+    trisfixed =  CUBE_TRIANGLES_CW
+
+    mnofix = fslmesh.Mesh(tris)
+    mfix   = fslmesh.Mesh(tris)
+
+    mnofix.addVertices(verts1, key='v1', fixWinding=False)
+    mnofix.addVertices(verts2, key='v2', fixWinding=False)
+    mfix  .addVertices(verts1, key='v1', fixWinding=True)
+    mfix  .addVertices(verts2, key='v2', fixWinding=True)
+
+    mnofix.vertices = 'v1'
+    assert np.all(mnofix.indices == tris)
+    mnofix.vertices = 'v2'
+    assert np.all(mnofix.indices == tris)
+
+    mfix.vertices = 'v1'
+    assert np.all(mfix.indices == tris)
+    mfix.vertices = 'v2'
+    assert np.all(mfix.indices == trisfixed)