From 1b7df5118daceaaff1253d327cc5df7f6e9cfb60 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Tue, 19 Sep 2023 15:08:17 -0400 Subject: [PATCH 01/24] 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 02/24] 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 03/24] 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 04/24] 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 05/24] 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 06/24] 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 07/24] 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 08/24] 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): From 9d5361a21f7534c0f2cde5dc96912dcea772bf80 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Tue, 19 Sep 2023 15:12:26 -0400 Subject: [PATCH 09/24] ENH: Add copy() method to ArrayProxy --- nibabel/arrayproxy.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nibabel/arrayproxy.py b/nibabel/arrayproxy.py index 12a0a7caf..f123e98d7 100644 --- a/nibabel/arrayproxy.py +++ b/nibabel/arrayproxy.py @@ -217,6 +217,15 @@ def __init__(self, file_like, spec, *, mmap=True, order=None, keep_file_open=Non ) self._lock = RLock() + def copy(self): + spec = self._shape, self._dtype, self._offset, self._slope, self._inter + return ArrayProxy( + self.file_like, + spec, + mmap=self._mmap, + keep_file_open=self._keep_file_open, + ) + def __del__(self): """If this ``ArrayProxy`` was created with ``keep_file_open=True``, the open file object is closed if necessary. From 81b103355123b48229f38d6652917b073c00224e Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 22 Sep 2023 08:25:35 -0400 Subject: [PATCH 10/24] ENH: Copy lock if filehandle is shared, add tests --- nibabel/arrayproxy.py | 39 ++++++++++++++++++++------------ nibabel/tests/test_arrayproxy.py | 28 +++++++++++++++++++---- 2 files changed, 48 insertions(+), 19 deletions(-) diff --git a/nibabel/arrayproxy.py b/nibabel/arrayproxy.py index f123e98d7..57d8aa0f8 100644 --- a/nibabel/arrayproxy.py +++ b/nibabel/arrayproxy.py @@ -58,6 +58,7 @@ if ty.TYPE_CHECKING: # pragma: no cover import numpy.typing as npt + from typing_extensions import Self # PY310 # Taken from numpy/__init__.pyi _DType = ty.TypeVar('_DType', bound=np.dtype[ty.Any]) @@ -212,19 +213,29 @@ def __init__(self, file_like, spec, *, mmap=True, order=None, keep_file_open=Non self.order = order # Flags to keep track of whether a single ImageOpener is created, and # whether a single underlying file handle is created. - self._keep_file_open, self._persist_opener = self._should_keep_file_open( - file_like, keep_file_open - ) + self._keep_file_open, self._persist_opener = self._should_keep_file_open(keep_file_open) self._lock = RLock() - def copy(self): + def _has_fh(self) -> bool: + """Determine if our file-like is a filehandle or path""" + return hasattr(self.file_like, 'read') and hasattr(self.file_like, 'seek') + + def copy(self) -> Self: + """Create a new ArrayProxy for the same file and parameters + + If the proxied file is an open file handle, the new ArrayProxy + will share a lock with the old one. + """ spec = self._shape, self._dtype, self._offset, self._slope, self._inter - return ArrayProxy( + new = self.__class__( self.file_like, spec, mmap=self._mmap, keep_file_open=self._keep_file_open, ) + if self._has_fh(): + new._lock = self._lock + return new def __del__(self): """If this ``ArrayProxy`` was created with ``keep_file_open=True``, @@ -245,13 +256,13 @@ def __setstate__(self, state): self.__dict__.update(state) self._lock = RLock() - def _should_keep_file_open(self, file_like, keep_file_open): + def _should_keep_file_open(self, keep_file_open): """Called by ``__init__``. This method determines how to manage ``ImageOpener`` instances, and the underlying file handles - the behaviour depends on: - - whether ``file_like`` is an an open file handle, or a path to a + - whether ``self.file_like`` is an an open file handle, or a path to a ``'.gz'`` file, or a path to a non-gzip file. - whether ``indexed_gzip`` is present (see :attr:`.openers.HAVE_INDEXED_GZIP`). @@ -270,24 +281,24 @@ def _should_keep_file_open(self, file_like, keep_file_open): and closed on each file access. The internal ``_keep_file_open`` flag is only relevant if - ``file_like`` is a ``'.gz'`` file, and the ``indexed_gzip`` library is + ``self.file_like`` is a ``'.gz'`` file, and the ``indexed_gzip`` library is present. This method returns the values to be used for the internal ``_persist_opener`` and ``_keep_file_open`` flags; these values are derived according to the following rules: - 1. If ``file_like`` is a file(-like) object, both flags are set to + 1. If ``self.file_like`` is a file(-like) object, both flags are set to ``False``. 2. If ``keep_file_open`` (as passed to :meth:``__init__``) is ``True``, both internal flags are set to ``True``. - 3. If ``keep_file_open`` is ``False``, but ``file_like`` is not a path + 3. If ``keep_file_open`` is ``False``, but ``self.file_like`` is not a path to a ``.gz`` file or ``indexed_gzip`` is not present, both flags are set to ``False``. - 4. If ``keep_file_open`` is ``False``, ``file_like`` is a path to a + 4. If ``keep_file_open`` is ``False``, ``self.file_like`` is a path to a ``.gz`` file, and ``indexed_gzip`` is present, ``_persist_opener`` is set to ``True``, and ``_keep_file_open`` is set to ``False``. In this case, file handle management is delegated to the @@ -296,8 +307,6 @@ def _should_keep_file_open(self, file_like, keep_file_open): Parameters ---------- - file_like : object - File-like object or filename, as passed to ``__init__``. keep_file_open : { True, False } Flag as passed to ``__init__``. @@ -320,10 +329,10 @@ def _should_keep_file_open(self, file_like, keep_file_open): raise ValueError('keep_file_open must be one of {None, True, False}') # file_like is a handle - keep_file_open is irrelevant - if hasattr(file_like, 'read') and hasattr(file_like, 'seek'): + if self._has_fh(): return False, False # if the file is a gzip file, and we have_indexed_gzip, - have_igzip = openers.HAVE_INDEXED_GZIP and file_like.endswith('.gz') + have_igzip = openers.HAVE_INDEXED_GZIP and self.file_like.endswith('.gz') persist_opener = keep_file_open or have_igzip return keep_file_open, persist_opener diff --git a/nibabel/tests/test_arrayproxy.py b/nibabel/tests/test_arrayproxy.py index 7558c55ea..be9ef1ba7 100644 --- a/nibabel/tests/test_arrayproxy.py +++ b/nibabel/tests/test_arrayproxy.py @@ -554,16 +554,36 @@ def test_keep_file_open_true_false_invalid(): ArrayProxy(fname, ((10, 10, 10), dtype)) +def islock(l): + # isinstance doesn't work on threading.Lock? + return hasattr(l, 'acquire') and hasattr(l, 'release') + + def test_pickle_lock(): # Test that ArrayProxy can be pickled, and that thread lock is created - def islock(l): - # isinstance doesn't work on threading.Lock? - return hasattr(l, 'acquire') and hasattr(l, 'release') - proxy = ArrayProxy('dummyfile', ((10, 10, 10), np.float32)) assert islock(proxy._lock) pickled = pickle.dumps(proxy) unpickled = pickle.loads(pickled) assert islock(unpickled._lock) assert proxy._lock is not unpickled._lock + + +def test_copy(): + # Test copying array proxies + + # If the file-like is a file name, get a new lock + proxy = ArrayProxy('dummyfile', ((10, 10, 10), np.float32)) + assert islock(proxy._lock) + copied = proxy.copy() + assert islock(copied._lock) + assert proxy._lock is not copied._lock + + # If an open filehandle, the lock should be shared to + # avoid changing filehandle state in critical sections + proxy = ArrayProxy(BytesIO(), ((10, 10, 10), np.float32)) + assert islock(proxy._lock) + copied = proxy.copy() + assert islock(copied._lock) + assert proxy._lock is copied._lock From 798f0c6fb773ecaba218b9f61868d583668f62cd Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 18 Feb 2022 08:45:26 -0500 Subject: [PATCH 11/24] ENH: Add stubs from BIAP 9 --- nibabel/coordimage.py | 61 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 nibabel/coordimage.py diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py new file mode 100644 index 000000000..9a74b0616 --- /dev/null +++ b/nibabel/coordimage.py @@ -0,0 +1,61 @@ +class CoordinateImage: + """ + Attributes + ---------- + header : a file-specific header + coordaxis : ``CoordinateAxis`` + dataobj : array-like + """ + + +class CoordinateAxis: + """ + Attributes + ---------- + parcels : list of ``Parcel`` objects + """ + + def load_structures(self, mapping): + """ + Associate parcels to ``Pointset`` structures + """ + raise NotImplementedError + + def __getitem__(self, slicer): + """ + Return a sub-sampled CoordinateAxis containing structures + matching the indices provided. + """ + raise NotImplementedError + + def get_indices(self, parcel, indices=None): + """ + Return the indices in the full axis that correspond to the + requested parcel. If indices are provided, further subsample + the requested parcel. + """ + raise NotImplementedError + + +class Parcel: + """ + Attributes + ---------- + name : str + structure : ``Pointset`` + indices : object that selects a subset of coordinates in structure + """ + + +class GeometryCollection: + """ + Attributes + ---------- + structures : dict + Mapping from structure names to ``Pointset`` + """ + + @classmethod + def from_spec(klass, pathlike): + """Load a collection of geometries from a specification.""" + raise NotImplementedError From 7a6d50c8567215349d424e6df38b5c5336e41d6d Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 21 Feb 2022 09:18:54 -0500 Subject: [PATCH 12/24] ENH: Implement CoordinateAxis. and Parcel.__getitem__ --- nibabel/coordimage.py | 54 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py index 9a74b0616..3eb3813b2 100644 --- a/nibabel/coordimage.py +++ b/nibabel/coordimage.py @@ -1,3 +1,6 @@ +from nibabel.fileslice import fill_slicer + + class CoordinateImage: """ Attributes @@ -7,6 +10,11 @@ class CoordinateImage: dataobj : array-like """ + def __init__(self, data, coordaxis, header=None): + self.data = data + self.coordaxis = coordaxis + self.header = header + class CoordinateAxis: """ @@ -15,6 +23,9 @@ class CoordinateAxis: parcels : list of ``Parcel`` objects """ + def __init__(self, parcels): + self.parcels = parcels + def load_structures(self, mapping): """ Associate parcels to ``Pointset`` structures @@ -26,7 +37,31 @@ def __getitem__(self, slicer): Return a sub-sampled CoordinateAxis containing structures matching the indices provided. """ - raise NotImplementedError + if slicer is Ellipsis or slicer == slice(None): + return self + elif isinstance(slicer, slice): + slicer = fill_slicer(slicer, len(self)) + print(slicer) + start, stop, step = slicer.start, slicer.stop, slicer.step + else: + raise TypeError(f'Indexing type not supported: {type(slicer)}') + + subparcels = [] + pstop = 0 + for parcel in self.parcels: + pstart, pstop = pstop, pstop + len(parcel) + print(pstart, pstop) + if pstop < start: + continue + if pstart >= stop: + break + if start < pstart: + substart = (start - pstart) % step + else: + substart = start - pstart + print(slice(substart, stop - pstart, step)) + subparcels.append(parcel[substart : stop - pstop : step]) + return CoordinateAxis(subparcels) def get_indices(self, parcel, indices=None): """ @@ -36,6 +71,9 @@ def get_indices(self, parcel, indices=None): """ raise NotImplementedError + def __len__(self): + return sum(len(parcel) for parcel in self.parcels) + class Parcel: """ @@ -46,6 +84,20 @@ class Parcel: indices : object that selects a subset of coordinates in structure """ + def __init__(self, name, structure, indices): + self.name = name + self.structure = structure + self.indices = indices + + def __repr__(self): + return f'' + + def __len__(self): + return len(self.indices) + + def __getitem__(self, slicer): + return self.__class__(self.name, self.structure, self.indices[slicer]) + class GeometryCollection: """ From 344bfd864d0c54c8e9bd1668f7c775e40540d795 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 21 Feb 2022 09:21:14 -0500 Subject: [PATCH 13/24] TEST: Add FreeSurferSubject geometry collection, test loading Cifti2 as cimg --- nibabel/tests/test_coordimage.py | 52 ++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 nibabel/tests/test_coordimage.py diff --git a/nibabel/tests/test_coordimage.py b/nibabel/tests/test_coordimage.py new file mode 100644 index 000000000..0c6d58bb9 --- /dev/null +++ b/nibabel/tests/test_coordimage.py @@ -0,0 +1,52 @@ +import os +from pathlib import Path + +import nibabel as nb +from nibabel import coordimage as ci +from nibabel import pointset as ps +from nibabel.tests.nibabel_data import get_nibabel_data + +from .test_pointset import FreeSurferHemisphere + +CIFTI2_DATA = Path(get_nibabel_data()) / 'nitest-cifti2' + + +class FreeSurferSubject(ci.GeometryCollection): + @classmethod + def from_subject(klass, subject_id, subjects_dir=None): + """Load a FreeSurfer subject by ID""" + if subjects_dir is None: + subjects_dir = os.environ['SUBJECTS_DIR'] + return klass.from_spec(Path(subjects_dir) / subject_id) + + @classmethod + def from_spec(klass, pathlike): + """Load a FreeSurfer subject from its directory structure""" + subject_dir = Path(pathlike) + surfs = subject_dir / 'surf' + structures = { + 'lh': FreeSurferHemisphere.from_filename(surfs / 'lh.white'), + 'rh': FreeSurferHemisphere.from_filename(surfs / 'rh.white'), + } + subject = super().__init__(structures) + subject._subject_dir = subject_dir + return subject + + +def test_Cifti2Image_as_CoordImage(): + ones = nb.load(CIFTI2_DATA / 'ones.dscalar.nii') + axes = [ones.header.get_axis(i) for i in range(ones.ndim)] + + parcels = [] + for name, slicer, bma in axes[1].iter_structures(): + if bma.volume_shape: + substruct = ps.NdGrid(bma.volume_shape, bma.affine) + indices = bma.voxel + else: + substruct = None + indices = bma.vertex + parcels.append(ci.Parcel(name, None, indices)) + caxis = ci.CoordinateAxis(parcels) + dobj = ones.dataobj.copy() + dobj.order = 'C' # Hack for image with BMA as the last axis + cimg = ci.CoordinateImage(dobj, caxis, ones.header) From 1138a957a8958853bb6e1adf4b7cbd6acd675d34 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Tue, 22 Feb 2022 15:46:53 -0500 Subject: [PATCH 14/24] ENH: Add CaretSpecFile type for use with CIFTI-2 --- nibabel/cifti2/caretspec.py | 199 ++++++++++++++++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 nibabel/cifti2/caretspec.py diff --git a/nibabel/cifti2/caretspec.py b/nibabel/cifti2/caretspec.py new file mode 100644 index 000000000..f0da1bbab --- /dev/null +++ b/nibabel/cifti2/caretspec.py @@ -0,0 +1,199 @@ +# emacs: -*- mode: python-mode; py-indent-offset: 4; indent-tabs-mode: nil -*- +# vi: set ft=python sts=4 ts=4 sw=4 et: +### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ## +# +# See COPYING file distributed along with the NiBabel package for the +# copyright and license terms. +# +### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ### ## +"""Read / write access to CaretSpecFile format + +The format of CaretSpecFiles does not seem to have any independent +documentation. + +Code can be found here [0], and a DTD was worked out in this email thread [1]. + +[0]: https://github.com/Washington-University/workbench/tree/master/src/Files +[1]: https://groups.google.com/a/humanconnectome.org/g/hcp-users/c/EGuwdaTVFuw/m/tg7a_-7mAQAJ +""" +import xml.etree.ElementTree as et + +from .. import xmlutils as xml +from ..caret import CaretMetaData + + +class CaretSpecDataFile(xml.XmlSerializable): + """DataFile + + * Attributes + + * Structure - A string from the BrainStructure list to identify + what structure this element refers to (usually left cortex, + right cortex, or cerebellum). + * DataFileType - A string from the DataFileType list + * Selected - A boolean + + * Child Elements: [NA] + * Text Content: A URI + * Parent Element - CaretSpecFile + + Attributes + ---------- + structure : str + Name of brain structure + data_file_type : str + Type of data file + selected : bool + Used for workbench internals + uri : str + URI of data file + """ + + def __init__(self, structure=None, data_file_type=None, selected=None, uri=None): + super().__init__() + self.structure = structure + self.data_file_type = data_file_type + self.selected = selected + self.uri = uri + + def _to_xml_element(self): + data_file = xml.Element('DataFile') + data_file.attrib['Structure'] = str(self.structure) + data_file.attrib['DataFileType'] = str(self.data_file_type) + data_file.attrib['Selected'] = 'true' if self.selected else 'false' + data_file.text = self.uri + return data_file + + def __repr__(self): + return self.to_xml().decode() + + +class CaretSpecFile(xml.XmlSerializable): + """Class for CaretSpecFile XML documents + + These are used to identify related surfaces and volumes for use with CIFTI-2 + data files. + """ + + def __init__(self, metadata=None, data_files=(), version='1.0'): + super().__init__() + if metadata is not None: + metadata = CaretMetaData(metadata) + self.metadata = metadata + self.data_files = list(data_files) + self.version = version + + def _to_xml_element(self): + caret_spec = xml.Element('CaretSpecFile') + caret_spec.attrib['Version'] = str(self.version) + if self.metadata is not None: + caret_spec.append(self.metadata._to_xml_element()) + for data_file in self.data_files: + caret_spec.append(data_file._to_xml_element()) + return caret_spec + + def to_xml(self, enc='UTF-8', **kwargs): + ele = self._to_xml_element() + et.indent(ele, ' ') + return et.tostring(ele, enc, xml_declaration=True, short_empty_elements=False, **kwargs) + + def __eq__(self, other): + return self.to_xml() == other.to_xml() + + @classmethod + def from_filename(klass, fname, **kwargs): + parser = CaretSpecParser(**kwargs) + with open(fname, 'rb') as fobj: + parser.parse(fptr=fobj) + return parser.caret_spec + + +class CaretSpecParser(xml.XmlParser): + def __init__(self, encoding=None, buffer_size=3500000, verbose=0): + super().__init__(encoding=encoding, buffer_size=buffer_size, verbose=verbose) + self.fsm_state = [] + self.struct_state = [] + + self.caret_spec = None + + # where to write CDATA: + self.write_to = None + + # Collecting char buffer fragments + self._char_blocks = [] + + def StartElementHandler(self, name, attrs): + self.flush_chardata() + if name == 'CaretSpecFile': + self.caret_spec = CaretSpecFile(version=attrs['Version']) + elif name == 'MetaData': + self.caret_spec.metadata = CaretMetaData() + elif name == 'MD': + self.fsm_state.append('MD') + self.struct_state.append(['', '']) + elif name in ('Name', 'Value'): + self.write_to = name + elif name == 'DataFile': + selected_map = {'true': True, 'false': False} + data_file = CaretSpecDataFile( + structure=attrs['Structure'], + data_file_type=attrs['DataFileType'], + selected=selected_map[attrs['Selected']], + ) + self.caret_spec.data_files.append(data_file) + self.struct_state.append(data_file) + self.write_to = 'DataFile' + + def EndElementHandler(self, name): + self.flush_chardata() + if name == 'CaretSpecFile': + ... + elif name == 'MetaData': + ... + elif name == 'MD': + key, value = self.struct_state.pop() + self.caret_spec.metadata[key] = value + elif name in ('Name', 'Value'): + self.write_to = None + elif name == 'DataFile': + self.struct_state.pop() + self.write_to = None + + def CharacterDataHandler(self, data): + """Collect character data chunks pending collation + + The parser breaks the data up into chunks of size depending on the + buffer_size of the parser. A large bit of character data, with standard + parser buffer_size (such as 8K) can easily span many calls to this + function. We thus collect the chunks and process them when we hit start + or end tags. + """ + if self._char_blocks is None: + self._char_blocks = [] + self._char_blocks.append(data) + + def flush_chardata(self): + """Collate and process collected character data""" + if self._char_blocks is None: + return + + # Just join the strings to get the data. Maybe there are some memory + # optimizations we could do by passing the list of strings to the + # read_data_block function. + data = ''.join(self._char_blocks) + # Reset the char collector + self._char_blocks = None + # Process data + if self.write_to == 'Name': + data = data.strip() # .decode('utf-8') + pair = self.struct_state[-1] + pair[0] = data + + elif self.write_to == 'Value': + data = data.strip() # .decode('utf-8') + pair = self.struct_state[-1] + pair[1] = data + + elif self.write_to == 'DataFile': + data = data.strip() + self.struct_state[-1].uri = data From c7ab610946e6935e3cf8c8fed3d2fa75ec755ba3 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Tue, 22 Feb 2022 15:54:14 -0500 Subject: [PATCH 15/24] TEST: Load CaretSpecFile as a GeometryCollection --- nibabel/coordimage.py | 3 +++ nibabel/tests/test_coordimage.py | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py index 3eb3813b2..fb629a3bd 100644 --- a/nibabel/coordimage.py +++ b/nibabel/coordimage.py @@ -107,6 +107,9 @@ class GeometryCollection: Mapping from structure names to ``Pointset`` """ + def __init__(self, structures): + self.structures = structures + @classmethod def from_spec(klass, pathlike): """Load a collection of geometries from a specification.""" diff --git a/nibabel/tests/test_coordimage.py b/nibabel/tests/test_coordimage.py index 0c6d58bb9..c20a4d235 100644 --- a/nibabel/tests/test_coordimage.py +++ b/nibabel/tests/test_coordimage.py @@ -28,11 +28,27 @@ def from_spec(klass, pathlike): 'lh': FreeSurferHemisphere.from_filename(surfs / 'lh.white'), 'rh': FreeSurferHemisphere.from_filename(surfs / 'rh.white'), } - subject = super().__init__(structures) + subject = klass(structures) subject._subject_dir = subject_dir return subject +class CaretSpec(ci.GeometryCollection): + @classmethod + def from_spec(klass, pathlike): + from nibabel.cifti2.caretspec import CaretSpecFile + + csf = CaretSpecFile.from_filename(pathlike) + structures = { + df.structure: df.uri + for df in csf.data_files + if df.selected # Use selected to avoid overloading for now + } + wbspec = klass(structures) + wbspec._specfile = csf + return wbspec + + def test_Cifti2Image_as_CoordImage(): ones = nb.load(CIFTI2_DATA / 'ones.dscalar.nii') axes = [ones.header.get_axis(i) for i in range(ones.ndim)] From a458ec3d72565add2f15faae64e0d81eb8bd3ab4 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 23 Feb 2022 10:08:55 -0500 Subject: [PATCH 16/24] ENH: Hacky mixin to make surface CaretDataFiles implement TriangularMesh --- nibabel/cifti2/caretspec.py | 39 +++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/nibabel/cifti2/caretspec.py b/nibabel/cifti2/caretspec.py index f0da1bbab..1b6e6de3a 100644 --- a/nibabel/cifti2/caretspec.py +++ b/nibabel/cifti2/caretspec.py @@ -17,9 +17,12 @@ [1]: https://groups.google.com/a/humanconnectome.org/g/hcp-users/c/EGuwdaTVFuw/m/tg7a_-7mAQAJ """ import xml.etree.ElementTree as et +from urllib.parse import urlparse -from .. import xmlutils as xml -from ..caret import CaretMetaData +import nibabel as nb +from nibabel import pointset as ps +from nibabel import xmlutils as xml +from nibabel.caret import CaretMetaData class CaretSpecDataFile(xml.XmlSerializable): @@ -56,6 +59,9 @@ def __init__(self, structure=None, data_file_type=None, selected=None, uri=None) self.selected = selected self.uri = uri + if data_file_type == 'SURFACE': + self.__class__ = SurfaceDataFile + def _to_xml_element(self): data_file = xml.Element('DataFile') data_file.attrib['Structure'] = str(self.structure) @@ -68,6 +74,35 @@ def __repr__(self): return self.to_xml().decode() +class SurfaceDataFile(ps.TriangularMesh, CaretSpecDataFile): + _gifti = None + _coords = None + _triangles = None + + def _get_gifti(self): + if self._gifti is None: + parts = urlparse(self.uri) + if parts.scheme == 'file': + self._gifti = nb.load(parts.path) + elif parts.scheme == '': + self._gifti = nb.load(self.uri) + else: + self._gifti = nb.GiftiImage.from_url(self.uri) + return self._gifti + + def get_triangles(self, name=None): + if self._triangles is None: + gifti = self._get_gifti() + self._triangles = gifti.agg_data('triangle') + return self._triangles + + def get_coords(self, name=None): + if self._coords is None: + gifti = self._get_gifti() + self._coords = gifti.agg_data('pointset') + return self._coords + + class CaretSpecFile(xml.XmlSerializable): """Class for CaretSpecFile XML documents From 921173bb58f003b9c3cfc94cf3ba938945b7e09c Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 23 Feb 2022 22:45:03 -0500 Subject: [PATCH 17/24] FIX: CoordinateAxis.__getitem__ fix --- nibabel/coordimage.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py index fb629a3bd..ee3699e40 100644 --- a/nibabel/coordimage.py +++ b/nibabel/coordimage.py @@ -41,7 +41,6 @@ def __getitem__(self, slicer): return self elif isinstance(slicer, slice): slicer = fill_slicer(slicer, len(self)) - print(slicer) start, stop, step = slicer.start, slicer.stop, slicer.step else: raise TypeError(f'Indexing type not supported: {type(slicer)}') @@ -50,7 +49,6 @@ def __getitem__(self, slicer): pstop = 0 for parcel in self.parcels: pstart, pstop = pstop, pstop + len(parcel) - print(pstart, pstop) if pstop < start: continue if pstart >= stop: @@ -59,8 +57,7 @@ def __getitem__(self, slicer): substart = (start - pstart) % step else: substart = start - pstart - print(slice(substart, stop - pstart, step)) - subparcels.append(parcel[substart : stop - pstop : step]) + subparcels.append(parcel[substart : stop - pstart : step]) return CoordinateAxis(subparcels) def get_indices(self, parcel, indices=None): From d4d42a02c536ca3c5cf84d8174d8fbbdfaa5a28d Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 23 Feb 2022 22:45:26 -0500 Subject: [PATCH 18/24] ENH: Implement CoordinateAxis.get_indices --- nibabel/coordimage.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py index ee3699e40..90980c877 100644 --- a/nibabel/coordimage.py +++ b/nibabel/coordimage.py @@ -1,3 +1,5 @@ +import numpy as np + from nibabel.fileslice import fill_slicer @@ -66,7 +68,19 @@ def get_indices(self, parcel, indices=None): requested parcel. If indices are provided, further subsample the requested parcel. """ - raise NotImplementedError + subseqs = [] + idx = 0 + for p in self.parcels: + if p.name == parcel: + subseqs.append(range(idx, idx + len(p))) + idx += len(p) + if not subseqs: + return () + if indices: + return np.hstack(subseqs)[indices] + if len(subseqs) == 1: + return subseqs[0] + return np.hstack(subseqs) def __len__(self): return sum(len(parcel) for parcel in self.parcels) From b51ec36c5916072520a8c1d3ce39fe9323d600f5 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 23 Feb 2022 22:45:45 -0500 Subject: [PATCH 19/24] TEST: Add some assertions and smoke tests to exercise methods --- nibabel/tests/test_coordimage.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/nibabel/tests/test_coordimage.py b/nibabel/tests/test_coordimage.py index c20a4d235..9f580b140 100644 --- a/nibabel/tests/test_coordimage.py +++ b/nibabel/tests/test_coordimage.py @@ -66,3 +66,18 @@ def test_Cifti2Image_as_CoordImage(): dobj = ones.dataobj.copy() dobj.order = 'C' # Hack for image with BMA as the last axis cimg = ci.CoordinateImage(dobj, caxis, ones.header) + + assert caxis[...] is caxis + assert caxis[:] is caxis + + subaxis = caxis[:100] + assert len(subaxis) == 100 + assert len(subaxis.parcels) == 1 + subaxis = caxis[100:] + assert len(subaxis) == len(caxis) - 100 + assert len(subaxis.parcels) == len(caxis.parcels) + subaxis = caxis[100:-100] + assert len(subaxis) == len(caxis) - 200 + assert len(subaxis.parcels) == len(caxis.parcels) + + caxis.get_indices('CIFTI_STRUCTURE_CORTEX_LEFT') From 76b52f50ebf28ab0f9823719ed330b3c7131b977 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Wed, 25 Jan 2023 10:27:10 -0500 Subject: [PATCH 20/24] ENH: Add from_image/from_header methods to bring logic out of tests --- nibabel/coordimage.py | 40 ++++++++++++++++++++++++++++++++ nibabel/tests/test_coordimage.py | 20 ++++------------ 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py index 90980c877..6602348de 100644 --- a/nibabel/coordimage.py +++ b/nibabel/coordimage.py @@ -1,5 +1,7 @@ import numpy as np +import nibabel as nib +import nibabel.pointset as ps from nibabel.fileslice import fill_slicer @@ -17,6 +19,22 @@ def __init__(self, data, coordaxis, header=None): self.coordaxis = coordaxis self.header = header + @classmethod + def from_image(klass, img): + coordaxis = CoordinateAxis.from_header(img.header) + if isinstance(img, nib.Cifti2Image): + if img.ndim != 2: + raise ValueError('Can only interpret 2D images') + for i in img.header.mapped_indices: + if isinstance(img.header.get_axis(i), nib.cifti2.BrainModelAxis): + break + # Reinterpret data ordering based on location of coordinate axis + data = img.dataobj.copy() + data.order = ['F', 'C'][i] + if i == 1: + data._shape = data._shape[::-1] + return klass(data, coordaxis, img.header) + class CoordinateAxis: """ @@ -85,6 +103,28 @@ def get_indices(self, parcel, indices=None): def __len__(self): return sum(len(parcel) for parcel in self.parcels) + # Hacky factory method for now + @classmethod + def from_header(klass, hdr): + parcels = [] + if isinstance(hdr, nib.Cifti2Header): + axes = [hdr.get_axis(i) for i in hdr.mapped_indices] + for ax in axes: + if isinstance(ax, nib.cifti2.BrainModelAxis): + break + else: + raise ValueError('No BrainModelAxis, cannot create CoordinateAxis') + for name, slicer, struct in ax.iter_structures(): + if struct.volume_shape: + substruct = ps.NdGrid(struct.volume_shape, struct.affine) + indices = struct.voxel + else: + substruct = None + indices = struct.vertex + parcels.append(Parcel(name, substruct, indices)) + + return klass(parcels) + class Parcel: """ diff --git a/nibabel/tests/test_coordimage.py b/nibabel/tests/test_coordimage.py index 9f580b140..fe0ac1c39 100644 --- a/nibabel/tests/test_coordimage.py +++ b/nibabel/tests/test_coordimage.py @@ -51,22 +51,12 @@ def from_spec(klass, pathlike): def test_Cifti2Image_as_CoordImage(): ones = nb.load(CIFTI2_DATA / 'ones.dscalar.nii') - axes = [ones.header.get_axis(i) for i in range(ones.ndim)] - - parcels = [] - for name, slicer, bma in axes[1].iter_structures(): - if bma.volume_shape: - substruct = ps.NdGrid(bma.volume_shape, bma.affine) - indices = bma.voxel - else: - substruct = None - indices = bma.vertex - parcels.append(ci.Parcel(name, None, indices)) - caxis = ci.CoordinateAxis(parcels) - dobj = ones.dataobj.copy() - dobj.order = 'C' # Hack for image with BMA as the last axis - cimg = ci.CoordinateImage(dobj, caxis, ones.header) + assert ones.shape == (1, 91282) + cimg = ci.CoordinateImage.from_image(ones) + assert cimg.shape == (91282, 1) + caxis = cimg.coordaxis + assert len(caxis) == 91282 assert caxis[...] is caxis assert caxis[:] is caxis From 1392a06a5fdec9b69e345020b35783ff0db613cc Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 24 Feb 2022 11:39:59 -0500 Subject: [PATCH 21/24] TEST: Add fsLR.wb.spec file for interpreting fsLR data --- nibabel/tests/data/fsLR.wb.spec | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 nibabel/tests/data/fsLR.wb.spec diff --git a/nibabel/tests/data/fsLR.wb.spec b/nibabel/tests/data/fsLR.wb.spec new file mode 100644 index 000000000..b7ad95831 --- /dev/null +++ b/nibabel/tests/data/fsLR.wb.spec @@ -0,0 +1,30 @@ + + + + + + https://raw.githubusercontent.com/mgxd/brainplot/master/brainplot/Conte69_Atlas/Conte69.L.midthickness.32k_fs_LR.surf.gii + + + https://raw.githubusercontent.com/mgxd/brainplot/master/brainplot/Conte69_Atlas/Conte69.R.midthickness.32k_fs_LR.surf.gii + + + https://raw.githubusercontent.com/mgxd/brainplot/master/brainplot/Conte69_Atlas/Conte69.L.very_inflated.32k_fs_LR.surf.gii + + + https://raw.githubusercontent.com/mgxd/brainplot/master/brainplot/Conte69_Atlas/Conte69.R.very_inflated.32k_fs_LR.surf.gii + + + https://templateflow.s3.amazonaws.com/tpl-MNI152NLin6Asym/tpl-MNI152NLin6Asym_res-02_T1w.nii.gz + + From 5edddd4e207eae3bde12a0457113adf29c5ce5a6 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Wed, 25 Jan 2023 10:28:13 -0500 Subject: [PATCH 22/24] ENH: Add CoordinateImage slicing by parcel name --- nibabel/coordimage.py | 20 +++++++++++++++++++- nibabel/tests/test_coordimage.py | 9 ++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/nibabel/coordimage.py b/nibabel/coordimage.py index 6602348de..d418d4743 100644 --- a/nibabel/coordimage.py +++ b/nibabel/coordimage.py @@ -19,6 +19,24 @@ def __init__(self, data, coordaxis, header=None): self.coordaxis = coordaxis self.header = header + @property + def shape(self): + return self.data.shape + + def __getitem__(self, slicer): + if isinstance(slicer, str): + slicer = self.coordaxis.get_indices(slicer) + elif isinstance(slicer, list): + slicer = np.hstack([self.coordaxis.get_indices(sub) for sub in slicer]) + + if isinstance(slicer, range): + slicer = slice(slicer.start, slicer.stop, slicer.step) + + data = self.data + if not isinstance(slicer, slice): + data = np.asanyarray(data) + return self.__class__(data[slicer], self.coordaxis[slicer], header=self.header.copy()) + @classmethod def from_image(klass, img): coordaxis = CoordinateAxis.from_header(img.header) @@ -57,7 +75,7 @@ def __getitem__(self, slicer): Return a sub-sampled CoordinateAxis containing structures matching the indices provided. """ - if slicer is Ellipsis or slicer == slice(None): + if slicer is Ellipsis or isinstance(slicer, slice) and slicer == slice(None): return self elif isinstance(slicer, slice): slicer = fill_slicer(slicer, len(self)) diff --git a/nibabel/tests/test_coordimage.py b/nibabel/tests/test_coordimage.py index fe0ac1c39..7318074f9 100644 --- a/nibabel/tests/test_coordimage.py +++ b/nibabel/tests/test_coordimage.py @@ -70,4 +70,11 @@ def test_Cifti2Image_as_CoordImage(): assert len(subaxis) == len(caxis) - 200 assert len(subaxis.parcels) == len(caxis.parcels) - caxis.get_indices('CIFTI_STRUCTURE_CORTEX_LEFT') + lh_img = cimg['CIFTI_STRUCTURE_CORTEX_LEFT'] + assert len(lh_img.coordaxis.parcels) == 1 + assert lh_img.shape == (29696, 1) + + # # Not working yet. + # cortex_img = cimg[["CIFTI_STRUCTURE_CORTEX_LEFT", "CIFTI_STRUCTURE_CORTEX_RIGHT"]] + # assert len(cortex_img.coordaxis.parcels) == 2 + # assert cortex_img.shape == (59412, 1) From 05ca9fb03c914f112ccdab8f472d8a64d43bdc6a Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 25 Feb 2022 08:10:08 -0500 Subject: [PATCH 23/24] RF: Simplify CaretSpecParser slightly --- nibabel/cifti2/caretspec.py | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/nibabel/cifti2/caretspec.py b/nibabel/cifti2/caretspec.py index 1b6e6de3a..6e32fb1d7 100644 --- a/nibabel/cifti2/caretspec.py +++ b/nibabel/cifti2/caretspec.py @@ -146,7 +146,6 @@ def from_filename(klass, fname, **kwargs): class CaretSpecParser(xml.XmlParser): def __init__(self, encoding=None, buffer_size=3500000, verbose=0): super().__init__(encoding=encoding, buffer_size=buffer_size, verbose=verbose) - self.fsm_state = [] self.struct_state = [] self.caret_spec = None @@ -164,8 +163,7 @@ def StartElementHandler(self, name, attrs): elif name == 'MetaData': self.caret_spec.metadata = CaretMetaData() elif name == 'MD': - self.fsm_state.append('MD') - self.struct_state.append(['', '']) + self.struct_state.append({}) elif name in ('Name', 'Value'): self.write_to = name elif name == 'DataFile': @@ -181,13 +179,9 @@ def StartElementHandler(self, name, attrs): def EndElementHandler(self, name): self.flush_chardata() - if name == 'CaretSpecFile': - ... - elif name == 'MetaData': - ... - elif name == 'MD': - key, value = self.struct_state.pop() - self.caret_spec.metadata[key] = value + if name == 'MD': + MD = self.struct_state.pop() + self.caret_spec.metadata[MD['Name']] = MD['Value'] elif name in ('Name', 'Value'): self.write_to = None elif name == 'DataFile': @@ -212,23 +206,12 @@ def flush_chardata(self): if self._char_blocks is None: return - # Just join the strings to get the data. Maybe there are some memory - # optimizations we could do by passing the list of strings to the - # read_data_block function. - data = ''.join(self._char_blocks) + data = ''.join(self._char_blocks).strip() # Reset the char collector self._char_blocks = None # Process data - if self.write_to == 'Name': - data = data.strip() # .decode('utf-8') - pair = self.struct_state[-1] - pair[0] = data - - elif self.write_to == 'Value': - data = data.strip() # .decode('utf-8') - pair = self.struct_state[-1] - pair[1] = data + if self.write_to in ('Name', 'Value'): + self.struct_state[-1][self.write_to] = data elif self.write_to == 'DataFile': - data = data.strip() self.struct_state[-1].uri = data From 6c2407d7b4037d783b585de2c5304a970e445d5b Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 20 Aug 2022 15:16:49 -0400 Subject: [PATCH 24/24] TEST: Check SurfaceDataFile retrieval --- nibabel/cifti2/tests/test_caretspec.py | 34 ++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 nibabel/cifti2/tests/test_caretspec.py diff --git a/nibabel/cifti2/tests/test_caretspec.py b/nibabel/cifti2/tests/test_caretspec.py new file mode 100644 index 000000000..604808c3f --- /dev/null +++ b/nibabel/cifti2/tests/test_caretspec.py @@ -0,0 +1,34 @@ +import unittest +from pathlib import Path + +from nibabel.cifti2.caretspec import * +from nibabel.optpkg import optional_package +from nibabel.testing import data_path + +requests, has_requests, _ = optional_package('requests') + + +def test_CaretSpecFile(): + fsLR = CaretSpecFile.from_filename(Path(data_path) / 'fsLR.wb.spec') + + assert fsLR.metadata == {} + assert fsLR.version == '1.0' + assert len(fsLR.data_files) == 5 + + for df in fsLR.data_files: + assert isinstance(df, CaretSpecDataFile) + if df.data_file_type == 'SURFACE': + assert isinstance(df, SurfaceDataFile) + + +@unittest.skipUnless(has_requests, reason='Test fetches from URL') +def test_SurfaceDataFile(): + fsLR = CaretSpecFile.from_filename(Path(data_path) / 'fsLR.wb.spec') + df = fsLR.data_files[0] + assert df.data_file_type == 'SURFACE' + try: + coords, triangles = df.get_mesh() + except IOError: + raise unittest.SkipTest(reason='Broken URL') + assert coords.shape == (32492, 3) + assert triangles.shape == (64980, 3)