From 1b7df5118daceaaff1253d327cc5df7f6e9cfb60 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Tue, 19 Sep 2023 15:08:17 -0400
Subject: [PATCH 1/8] ENH: Start restoring triangular meshes
---
nibabel/pointset.py | 70 +++++++++++++++
nibabel/tests/test_pointset.py | 156 +++++++++++++++++++++++++++++++++
2 files changed, 226 insertions(+)
diff --git a/nibabel/pointset.py b/nibabel/pointset.py
index b40449801..3f90c4fa0 100644
--- a/nibabel/pointset.py
+++ b/nibabel/pointset.py
@@ -48,6 +48,11 @@ def __array__(self, dtype: _DType, /) -> np.ndarray[ty.Any, _DType]:
... # pragma: no cover
+class HasMeshAttrs(ty.Protocol):
+ coordinates: CoordinateArray
+ triangles: CoordinateArray
+
+
@dataclass
class Pointset:
"""A collection of points described by coordinates.
@@ -144,6 +149,71 @@ def get_coords(self, *, as_homogeneous: bool = False):
return coords
+class TriangularMesh(Pointset):
+ triangles: CoordinateArray
+
+ def __init__(
+ self,
+ coordinates: CoordinateArray,
+ triangles: CoordinateArray,
+ affine: np.ndarray | None = None,
+ homogeneous: bool = False,
+ ):
+ super().__init__(coordinates, affine=affine, homogeneous=homogeneous)
+ self.triangles = triangles
+
+ @classmethod
+ def from_tuple(
+ cls,
+ mesh: tuple[CoordinateArray, CoordinateArray],
+ affine: np.ndarray | None = None,
+ homogeneous: bool = False,
+ ) -> Self:
+ return cls(mesh[0], mesh[1], affine=affine, homogeneous=homogeneous)
+
+ @classmethod
+ def from_object(
+ cls,
+ mesh: HasMeshAttrs,
+ affine: np.ndarray | None = None,
+ homogeneous: bool = False,
+ ) -> Self:
+ return cls(mesh.coordinates, mesh.triangles, affine=affine, homogeneous=homogeneous)
+
+ @property
+ def n_triangles(self):
+ """Number of faces
+
+ Subclasses should override with more efficient implementations.
+ """
+ return self.triangles.shape[0]
+
+ def get_triangles(self):
+ """Mx3 array of indices into coordinate table"""
+ return np.asanyarray(self.triangles)
+
+ def get_mesh(self, *, as_homogeneous: bool = False):
+ return self.get_coords(as_homogeneous=as_homogeneous), self.get_triangles()
+
+
+class CoordinateFamilyMixin(Pointset):
+ def __init__(self, *args, **kwargs):
+ self._coords = {}
+ super().__init__(*args, **kwargs)
+
+ def get_names(self):
+ """List of surface names that can be passed to :meth:`with_name`"""
+ return list(self._coords)
+
+ def with_name(self, name: str) -> Self:
+ new = replace(self, coordinates=self._coords[name])
+ new._coords = self._coords
+ return new
+
+ def add_coordinates(self, name, coordinates):
+ self._coords[name] = coordinates
+
+
class Grid(Pointset):
r"""A regularly-spaced collection of coordinates
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index fb9a7c5c8..1a28828c4 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -182,3 +182,159 @@ def test_to_mask(self):
],
)
assert np.array_equal(mask_img.affine, np.eye(4))
+
+
+class TestTriangularMeshes(TestPointsets):
+ ...
+
+
+class H5ArrayProxy:
+ def __init__(self, file_like, dataset_name):
+ self.file_like = file_like
+ self.dataset_name = dataset_name
+ with h5.File(file_like, 'r') as h5f:
+ arr = h5f[dataset_name]
+ self._shape = arr.shape
+ self._dtype = arr.dtype
+
+ @property
+ def is_proxy(self):
+ return True
+
+ @property
+ def shape(self):
+ return self._shape
+
+ @property
+ def ndim(self):
+ return len(self.shape)
+
+ @property
+ def dtype(self):
+ return self._dtype
+
+ def __array__(self, dtype=None):
+ with h5.File(self.file_like, 'r') as h5f:
+ return np.asanyarray(h5f[self.dataset_name], dtype)
+
+ def __getitem__(self, slicer):
+ with h5.File(self.file_like, 'r') as h5f:
+ return h5f[self.dataset_name][slicer]
+
+
+class H5Geometry(ps.TriMeshFamily):
+ """Simple Geometry file structure that combines a single topology
+ with one or more coordinate sets
+ """
+
+ @classmethod
+ def from_filename(klass, pathlike):
+ meshes = {}
+ with h5.File(pathlike, 'r') as h5f:
+ triangles = H5ArrayProxy(pathlike, '/topology')
+ for name in h5f['coordinates']:
+ meshes[name] = (H5ArrayProxy(pathlike, f'/coordinates/{name}'), triangles)
+ return klass(meshes)
+
+ def to_filename(self, pathlike):
+ with h5.File(pathlike, 'w') as h5f:
+ h5f.create_dataset('/topology', data=self.get_triangles())
+ for name, coord in self._coords.items():
+ h5f.create_dataset(f'/coordinates/{name}', data=coord)
+
+
+class FSGeometryProxy:
+ def __init__(self, pathlike):
+ self._file_like = str(Path(pathlike))
+ self._offset = None
+ self._vnum = None
+ self._fnum = None
+
+ def _peek(self):
+ from nibabel.freesurfer.io import _fread3
+
+ with open(self._file_like, 'rb') as fobj:
+ magic = _fread3(fobj)
+ if magic != 16777214:
+ raise NotImplementedError('Triangle files only!')
+ fobj.readline()
+ fobj.readline()
+ self._vnum = np.fromfile(fobj, '>i4', 1)[0]
+ self._fnum = np.fromfile(fobj, '>i4', 1)[0]
+ self._offset = fobj.tell()
+
+ @property
+ def vnum(self):
+ if self._vnum is None:
+ self._peek()
+ return self._vnum
+
+ @property
+ def fnum(self):
+ if self._fnum is None:
+ self._peek()
+ return self._fnum
+
+ @property
+ def offset(self):
+ if self._offset is None:
+ self._peek()
+ return self._offset
+
+ @auto_attr
+ def coords(self):
+ ap = ArrayProxy(self._file_like, ((self.vnum, 3), '>f4', self.offset))
+ ap.order = 'C'
+ return ap
+
+ @auto_attr
+ def triangles(self):
+ offset = self.offset + 12 * self.vnum
+ ap = ArrayProxy(self._file_like, ((self.fnum, 3), '>i4', offset))
+ ap.order = 'C'
+ return ap
+
+
+class FreeSurferHemisphere(ps.TriMeshFamily):
+ @classmethod
+ def from_filename(klass, pathlike):
+ path = Path(pathlike)
+ hemi, default = path.name.split('.')
+ mesh_names = (
+ 'orig',
+ 'white',
+ 'smoothwm',
+ 'pial',
+ 'inflated',
+ 'sphere',
+ 'midthickness',
+ 'graymid',
+ ) # Often created
+ if default not in mesh_names:
+ mesh_names.append(default)
+ meshes = {}
+ for mesh in mesh_names:
+ fpath = path.parent / f'{hemi}.{mesh}'
+ if fpath.exists():
+ meshes[mesh] = FSGeometryProxy(fpath)
+ hemi = klass(meshes)
+ hemi._default = default
+ return hemi
+
+
+def test_FreeSurferHemisphere():
+ lh = FreeSurferHemisphere.from_filename(FS_DATA / 'fsaverage/surf/lh.white')
+ assert lh.n_coords == 163842
+ assert lh.n_triangles == 327680
+
+
+@skipUnless(has_h5py, reason='Test requires h5py')
+def test_make_H5Geometry(tmp_path):
+ lh = FreeSurferHemisphere.from_filename(FS_DATA / 'fsaverage/surf/lh.white')
+ h5geo = H5Geometry({name: lh.get_mesh(name) for name in ('white', 'pial')})
+ h5geo.to_filename(tmp_path / 'geometry.h5')
+
+ rt_h5geo = H5Geometry.from_filename(tmp_path / 'geometry.h5')
+ assert set(h5geo._coords) == set(rt_h5geo._coords)
+ assert np.array_equal(lh.get_coords('white'), rt_h5geo.get_coords('white'))
+ assert np.array_equal(lh.get_triangles(), rt_h5geo.get_triangles())
From 5d25cef5ec60e5294628277b347c55c9f47b5a37 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Wed, 20 Sep 2023 10:02:41 -0400
Subject: [PATCH 2/8] TEST: Test basic TriangularMesh behavior
---
nibabel/pointset.py | 5 ++--
nibabel/tests/test_pointset.py | 44 ++++++++++++++++++++++++++++++++--
2 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/nibabel/pointset.py b/nibabel/pointset.py
index 3f90c4fa0..2664227c7 100644
--- a/nibabel/pointset.py
+++ b/nibabel/pointset.py
@@ -53,7 +53,7 @@ class HasMeshAttrs(ty.Protocol):
triangles: CoordinateArray
-@dataclass
+@dataclass(init=False)
class Pointset:
"""A collection of points described by coordinates.
@@ -70,7 +70,7 @@ class Pointset:
coordinates: CoordinateArray
affine: np.ndarray
- homogeneous: bool = False
+ homogeneous: bool
# Force use of __rmatmul__ with numpy arrays
__array_priority__ = 99
@@ -149,6 +149,7 @@ def get_coords(self, *, as_homogeneous: bool = False):
return coords
+@dataclass(init=False)
class TriangularMesh(Pointset):
triangles: CoordinateArray
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 1a28828c4..5cf3b8094 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -1,3 +1,4 @@
+from collections import namedtuple
from math import prod
from pathlib import Path
from unittest import skipUnless
@@ -184,8 +185,47 @@ def test_to_mask(self):
assert np.array_equal(mask_img.affine, np.eye(4))
-class TestTriangularMeshes(TestPointsets):
- ...
+class TestTriangularMeshes:
+ def test_init(self):
+ # Tetrahedron
+ coords = np.array(
+ [
+ [0.0, 0.0, 0.0],
+ [0.0, 0.0, 1.0],
+ [0.0, 1.0, 0.0],
+ [1.0, 0.0, 0.0],
+ ]
+ )
+ triangles = np.array(
+ [
+ [0, 2, 1],
+ [0, 3, 2],
+ [0, 1, 3],
+ [1, 2, 3],
+ ]
+ )
+
+ mesh = namedtuple('mesh', ('coordinates', 'triangles'))(coords, triangles)
+
+ tm1 = ps.TriangularMesh(coords, triangles)
+ tm2 = ps.TriangularMesh.from_tuple(mesh)
+ tm3 = ps.TriangularMesh.from_object(mesh)
+
+ assert np.allclose(tm1.affine, np.eye(4))
+ assert np.allclose(tm2.affine, np.eye(4))
+ assert np.allclose(tm3.affine, np.eye(4))
+
+ assert tm1.homogeneous is False
+ assert tm2.homogeneous is False
+ assert tm3.homogeneous is False
+
+ assert (tm1.n_coords, tm1.dim) == (4, 3)
+ assert (tm2.n_coords, tm2.dim) == (4, 3)
+ assert (tm3.n_coords, tm3.dim) == (4, 3)
+
+ assert tm1.n_triangles == 4
+ assert tm2.n_triangles == 4
+ assert tm3.n_triangles == 4
class H5ArrayProxy:
From e426afef9f9ea60bb05ef8525f358a1b173d878a Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Wed, 20 Sep 2023 10:03:55 -0400
Subject: [PATCH 3/8] RF: Use order kwarg for array proxies
---
nibabel/tests/test_pointset.py | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 5cf3b8094..1572e3209 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -322,17 +322,16 @@ def offset(self):
return self._offset
@auto_attr
- def coords(self):
- ap = ArrayProxy(self._file_like, ((self.vnum, 3), '>f4', self.offset))
- ap.order = 'C'
- return ap
+ def coordinates(self):
+ return ArrayProxy(self._file_like, ((self.vnum, 3), '>f4', self.offset), order='C')
@auto_attr
def triangles(self):
- offset = self.offset + 12 * self.vnum
- ap = ArrayProxy(self._file_like, ((self.fnum, 3), '>i4', offset))
- ap.order = 'C'
- return ap
+ return ArrayProxy(
+ self._file_like,
+ ((self.fnum, 3), '>i4', self.offset + 12 * self.vnum),
+ order='C',
+ )
class FreeSurferHemisphere(ps.TriMeshFamily):
From 1a46c7003633196486854f10d76c7f902b818b32 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Wed, 20 Sep 2023 10:04:51 -0400
Subject: [PATCH 4/8] TEST: Rework FreeSurferHemisphere and H5Geometry examples
with mixin
---
nibabel/tests/test_pointset.py | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 1572e3209..fba1e38e9 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -262,19 +262,21 @@ def __getitem__(self, slicer):
return h5f[self.dataset_name][slicer]
-class H5Geometry(ps.TriMeshFamily):
+class H5Geometry(ps.TriangularMesh, ps.CoordinateFamilyMixin):
"""Simple Geometry file structure that combines a single topology
with one or more coordinate sets
"""
@classmethod
def from_filename(klass, pathlike):
- meshes = {}
+ coords = {}
with h5.File(pathlike, 'r') as h5f:
triangles = H5ArrayProxy(pathlike, '/topology')
for name in h5f['coordinates']:
- meshes[name] = (H5ArrayProxy(pathlike, f'/coordinates/{name}'), triangles)
- return klass(meshes)
+ coords[name] = H5ArrayProxy(pathlike, f'/coordinates/{name}')
+ self = klass(next(iter(coords.values())), triangles)
+ self._coords.update(coords)
+ return self
def to_filename(self, pathlike):
with h5.File(pathlike, 'w') as h5f:
@@ -334,11 +336,13 @@ def triangles(self):
)
-class FreeSurferHemisphere(ps.TriMeshFamily):
+class FreeSurferHemisphere(ps.TriangularMesh, ps.CoordinateFamilyMixin):
@classmethod
def from_filename(klass, pathlike):
path = Path(pathlike)
hemi, default = path.name.split('.')
+ self = klass.from_object(FSGeometryProxy(path))
+ self._coords[default] = self.coordinates
mesh_names = (
'orig',
'white',
@@ -349,16 +353,11 @@ def from_filename(klass, pathlike):
'midthickness',
'graymid',
) # Often created
- if default not in mesh_names:
- mesh_names.append(default)
- meshes = {}
for mesh in mesh_names:
fpath = path.parent / f'{hemi}.{mesh}'
- if fpath.exists():
- meshes[mesh] = FSGeometryProxy(fpath)
- hemi = klass(meshes)
- hemi._default = default
- return hemi
+ if mesh not in self._coords and fpath.exists():
+ self.add_coordinates(mesh, FSGeometryProxy(fpath).coordinates)
+ return self
def test_FreeSurferHemisphere():
@@ -370,10 +369,14 @@ def test_FreeSurferHemisphere():
@skipUnless(has_h5py, reason='Test requires h5py')
def test_make_H5Geometry(tmp_path):
lh = FreeSurferHemisphere.from_filename(FS_DATA / 'fsaverage/surf/lh.white')
- h5geo = H5Geometry({name: lh.get_mesh(name) for name in ('white', 'pial')})
+ h5geo = H5Geometry.from_object(lh)
+ for name in ('white', 'pial'):
+ h5geo.add_coordinates(name, lh.with_name(name).coordinates)
h5geo.to_filename(tmp_path / 'geometry.h5')
rt_h5geo = H5Geometry.from_filename(tmp_path / 'geometry.h5')
assert set(h5geo._coords) == set(rt_h5geo._coords)
- assert np.array_equal(lh.get_coords('white'), rt_h5geo.get_coords('white'))
+ assert np.array_equal(
+ lh.with_name('white').get_coords(), rt_h5geo.with_name('white').get_coords()
+ )
assert np.array_equal(lh.get_triangles(), rt_h5geo.get_triangles())
From be05f0917ba27ac3d3147343b757cc9b9f88e8c5 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Wed, 20 Sep 2023 13:00:50 -0400
Subject: [PATCH 5/8] TEST: Tag tests that require access to the data directory
---
nibabel/tests/test_pointset.py | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index fba1e38e9..35e6272d2 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -13,7 +13,7 @@
from nibabel.onetime import auto_attr
from nibabel.optpkg import optional_package
from nibabel.spatialimages import SpatialImage
-from nibabel.tests.nibabel_data import get_nibabel_data
+from nibabel.tests.nibabel_data import get_nibabel_data, needs_nibabel_data
h5, has_h5py, _ = optional_package('h5py')
@@ -360,6 +360,7 @@ def from_filename(klass, pathlike):
return self
+@needs_nibabel_data('nitest-freesurfer')
def test_FreeSurferHemisphere():
lh = FreeSurferHemisphere.from_filename(FS_DATA / 'fsaverage/surf/lh.white')
assert lh.n_coords == 163842
@@ -367,6 +368,7 @@ def test_FreeSurferHemisphere():
@skipUnless(has_h5py, reason='Test requires h5py')
+@needs_nibabel_data('nitest-freesurfer')
def test_make_H5Geometry(tmp_path):
lh = FreeSurferHemisphere.from_filename(FS_DATA / 'fsaverage/surf/lh.white')
h5geo = H5Geometry.from_object(lh)
From cbb91d1f9eb8a5768190bc313f82755f83c7df94 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Fri, 22 Sep 2023 08:50:53 -0400
Subject: [PATCH 6/8] RF: Allow coordinate names to be set on init
---
nibabel/pointset.py | 13 +++++++++----
nibabel/tests/test_pointset.py | 18 +++++++++---------
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/nibabel/pointset.py b/nibabel/pointset.py
index 2664227c7..fed5513b4 100644
--- a/nibabel/pointset.py
+++ b/nibabel/pointset.py
@@ -169,8 +169,9 @@ def from_tuple(
mesh: tuple[CoordinateArray, CoordinateArray],
affine: np.ndarray | None = None,
homogeneous: bool = False,
+ **kwargs,
) -> Self:
- return cls(mesh[0], mesh[1], affine=affine, homogeneous=homogeneous)
+ return cls(mesh[0], mesh[1], affine=affine, homogeneous=homogeneous, **kwargs)
@classmethod
def from_object(
@@ -178,8 +179,11 @@ def from_object(
mesh: HasMeshAttrs,
affine: np.ndarray | None = None,
homogeneous: bool = False,
+ **kwargs,
) -> Self:
- return cls(mesh.coordinates, mesh.triangles, affine=affine, homogeneous=homogeneous)
+ return cls(
+ mesh.coordinates, mesh.triangles, affine=affine, homogeneous=homogeneous, **kwargs
+ )
@property
def n_triangles(self):
@@ -198,9 +202,10 @@ def get_mesh(self, *, as_homogeneous: bool = False):
class CoordinateFamilyMixin(Pointset):
- def __init__(self, *args, **kwargs):
- self._coords = {}
+ def __init__(self, *args, name='original', **kwargs):
+ mapping = kwargs.pop('mapping', {})
super().__init__(*args, **kwargs)
+ self._coords = {name: self.coordinates, **mapping}
def get_names(self):
"""List of surface names that can be passed to :meth:`with_name`"""
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 35e6272d2..2b7e0fd30 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -262,7 +262,7 @@ def __getitem__(self, slicer):
return h5f[self.dataset_name][slicer]
-class H5Geometry(ps.TriangularMesh, ps.CoordinateFamilyMixin):
+class H5Geometry(ps.CoordinateFamilyMixin, ps.TriangularMesh):
"""Simple Geometry file structure that combines a single topology
with one or more coordinate sets
"""
@@ -274,8 +274,7 @@ def from_filename(klass, pathlike):
triangles = H5ArrayProxy(pathlike, '/topology')
for name in h5f['coordinates']:
coords[name] = H5ArrayProxy(pathlike, f'/coordinates/{name}')
- self = klass(next(iter(coords.values())), triangles)
- self._coords.update(coords)
+ self = klass(next(iter(coords.values())), triangles, mapping=coords)
return self
def to_filename(self, pathlike):
@@ -336,13 +335,12 @@ def triangles(self):
)
-class FreeSurferHemisphere(ps.TriangularMesh, ps.CoordinateFamilyMixin):
+class FreeSurferHemisphere(ps.CoordinateFamilyMixin, ps.TriangularMesh):
@classmethod
def from_filename(klass, pathlike):
path = Path(pathlike)
hemi, default = path.name.split('.')
- self = klass.from_object(FSGeometryProxy(path))
- self._coords[default] = self.coordinates
+ self = klass.from_object(FSGeometryProxy(path), name=default)
mesh_names = (
'orig',
'white',
@@ -353,10 +351,12 @@ def from_filename(klass, pathlike):
'midthickness',
'graymid',
) # Often created
+
for mesh in mesh_names:
- fpath = path.parent / f'{hemi}.{mesh}'
- if mesh not in self._coords and fpath.exists():
- self.add_coordinates(mesh, FSGeometryProxy(fpath).coordinates)
+ if mesh != default:
+ fpath = path.parent / f'{hemi}.{mesh}'
+ if fpath.exists():
+ self.add_coordinates(mesh, FSGeometryProxy(fpath).coordinates)
return self
From 107ead6899a3e79b8aa9d958d71ef3683f16f373 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Fri, 22 Sep 2023 08:51:15 -0400
Subject: [PATCH 7/8] TEST: More fully test mesh and family APIs
---
nibabel/tests/test_pointset.py | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 2b7e0fd30..2a0b54561 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -186,7 +186,7 @@ def test_to_mask(self):
class TestTriangularMeshes:
- def test_init(self):
+ def test_api(self):
# Tetrahedron
coords = np.array(
[
@@ -227,6 +227,36 @@ def test_init(self):
assert tm2.n_triangles == 4
assert tm3.n_triangles == 4
+ out_coords, out_tris = tm1.get_mesh()
+ # Currently these are the exact arrays, but I don't think we should
+ # bake that assumption into the tests
+ assert np.allclose(out_coords, coords)
+ assert np.allclose(out_tris, triangles)
+
+
+class TestCoordinateFamilyMixin(TestPointsets):
+ def test_names(self):
+ coords = np.array(
+ [
+ [0.0, 0.0, 0.0],
+ [0.0, 0.0, 1.0],
+ [0.0, 1.0, 0.0],
+ [1.0, 0.0, 0.0],
+ ]
+ )
+ cfm = ps.CoordinateFamilyMixin(coords)
+
+ assert cfm.get_names() == ['original']
+ assert np.allclose(cfm.with_name('original').coordinates, coords)
+
+ cfm.add_coordinates('shifted', coords + 1)
+ assert cfm.get_names() == ['original', 'shifted']
+ shifted = cfm.with_name('shifted')
+ assert np.allclose(shifted.coordinates, coords + 1)
+ assert shifted.get_names() == ['original', 'shifted']
+ original = shifted.with_name('original')
+ assert np.allclose(original.coordinates, coords)
+
class H5ArrayProxy:
def __init__(self, file_like, dataset_name):
From 368c145543dd7d0742148fea55eeafa32dd69640 Mon Sep 17 00:00:00 2001
From: Chris Markiewicz
Date: Fri, 22 Sep 2023 09:50:29 -0400
Subject: [PATCH 8/8] ENH: Avoid duplicating objects, note that coordinate
family mappings are shared after with_name()
---
nibabel/pointset.py | 7 ++++++-
nibabel/tests/test_pointset.py | 19 +++++++++++++++++--
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/nibabel/pointset.py b/nibabel/pointset.py
index fed5513b4..db70f815d 100644
--- a/nibabel/pointset.py
+++ b/nibabel/pointset.py
@@ -212,7 +212,12 @@ def get_names(self):
return list(self._coords)
def with_name(self, name: str) -> Self:
- new = replace(self, coordinates=self._coords[name])
+ new_coords = self._coords[name]
+ if new_coords is self.coordinates:
+ return self
+ # Make a copy, preserving all dataclass fields
+ new = replace(self, coordinates=new_coords)
+ # Conserve exact _coords mapping
new._coords = self._coords
return new
diff --git a/nibabel/tests/test_pointset.py b/nibabel/tests/test_pointset.py
index 2a0b54561..31c5335a1 100644
--- a/nibabel/tests/test_pointset.py
+++ b/nibabel/tests/test_pointset.py
@@ -250,13 +250,28 @@ def test_names(self):
assert np.allclose(cfm.with_name('original').coordinates, coords)
cfm.add_coordinates('shifted', coords + 1)
- assert cfm.get_names() == ['original', 'shifted']
+ assert set(cfm.get_names()) == {'original', 'shifted'}
shifted = cfm.with_name('shifted')
assert np.allclose(shifted.coordinates, coords + 1)
- assert shifted.get_names() == ['original', 'shifted']
+ assert set(shifted.get_names()) == {'original', 'shifted'}
original = shifted.with_name('original')
assert np.allclose(original.coordinates, coords)
+ # Avoid duplicating objects
+ assert original.with_name('original') is original
+ # But don't try too hard
+ assert original.with_name('original') is not cfm
+
+ # with_name() preserves the exact coordinate mapping of the source object.
+ # Modifications of one are immediately available to all others.
+ # This is currently an implementation detail, and the expectation is that
+ # a family will be created once and then queried, but this behavior could
+ # potentially become confusing or relied upon.
+ # Change with care.
+ shifted.add_coordinates('shifted-again', coords + 2)
+ shift2 = shifted.with_name('shifted-again')
+ shift3 = cfm.with_name('shifted-again')
+
class H5ArrayProxy:
def __init__(self, file_like, dataset_name):