-
Notifications
You must be signed in to change notification settings - Fork 258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Add pointset data structures [BIAP9] #1251
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1251 +/- ##
==========================================
+ Coverage 92.16% 92.21% +0.04%
==========================================
Files 98 99 +1
Lines 12371 12453 +82
Branches 2542 2559 +17
==========================================
+ Hits 11402 11483 +81
Misses 646 646
- Partials 323 324 +1
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions, but it looks good to me overall
@oesteban What do you think about adding an class Pointset:
def __init__(self, coordinates, affine, is_homogeneous): ...
def __rmatmul__(self, affine):
"""Transform coordinates without evaluating"""
assert is_affine(affine)
return replace(self, affine @ self.affine)
def as_homogeneous(self):
if self.is_homogeneous:
return self.coordinates
return np.hstack(self.coordinates, np.ones(...))
def get_coords(self, homogeneous=False):
"""Evaluate coordinates"""
coords = self.affine @ self.as_homogeneous()
if not homogeneous:
coords = coords[:, :-1]
return coords Then transforming can simply be: transformed = xfm @ pointset (It takes a little bit more to get this actually to work.)from dataclasses import dataclass, replace
import numpy as np
@dataclass
class Pointset:
coordinates: np.ndarray
affine: np.ndarray
is_homogeneous: bool
def __rmatmul__(self, affine):
"""Transform coordinates without evaluating"""
assert is_affine(affine)
return replace(self, affine=affine @ self.affine)
def as_homogeneous(self):
if self.is_homogeneous:
return self.coordinates
return np.hstack(self.coordinates, np.ones((self.coordinates.shape[0], 1)))
def get_coords(self, homogeneous=False):
"""Evaluate coordinates"""
coords = self.affine @ self.as_homogeneous()
if not homogeneous:
coords = coords[:, :-1]
return coords
# Trick numpy into using our __rmatmul__
ndim = 2
__array_priority__ = 99
def is_affine(arr):
return arr.shape == (4, 4) and np.array_equal(arr[3], [0, 0, 0, 1]) This would work for clouds, meshes or grids and would let us add format-defined affines without applying them prematurely. And for grids, it would allow us to wait to generate the voxel indices. I would not attempt to apply warps with |
2607f17
to
af4e08a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
I've gone ahead and pushed forward on bundling affines with Updated comparisons to
LMK what you think. It can be reverted, if this doesn't make sense to anybody else. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. I left a few thoughts, but nothing stood out in my fast pass
Co-authored-by: Oscar Esteban <[email protected]>
Co-authored-by: Oscar Esteban <[email protected]>
6ca6d9a
to
5ded851
Compare
I've removed the triangular meshes from this for now. The existing architecture depended too much on the previous |
Breaking this section out of #1090, as these will be useful for the transformation effort, as well as surfaces.
We will want factory functions or classmethods to generate these types from some common structures, such as
SpatialImage
,GiftiDataArray
or FreeSurfer geometry arrays (which are just numpy arrays).We may also consider pulling
Parcel
out of the CoordinateImage PR, for the sake of more efficiently handling subsets of coordinates.nibabel/nibabel/coordimage.py
Lines 147 to 168 in 42e712e
This API is up for debate. I'm not sure I like the inclusion of named spaces and default spaces anymore. And we may want to expand these from such bare bones, though I think we should be careful of relying too much on an object.
Mapping/comparison to nitransforms concepts:
Pointset
=SampledSpatialData
SampledSpatialData
also includes the dimensionality of the points (generally 3)NDGrid
=ImageGrid
ImageGrid().ndindex
corresponds toNDGrid().get_coords('voxels')
ImageGrid().ndcoords
corresponds toNDGrid().get_coords()
ImageGrid
preserves an original image header and pre-calculates an inverse affineImageGrid().ras()
andImageGrid().index()
apply the affine (or inverse) to passed indices/coordinates