- Notifications
You must be signed in to change notification settings - Fork269
ENH: Add pointset data structures [BIAP9]#1251
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedSep 7, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
@oesteban What do you think about adding an classPointset:def__init__(self,coordinates,affine,is_homogeneous): ...def__rmatmul__(self,affine):"""Transform coordinates without evaluating"""assertis_affine(affine)returnreplace(self,affine @self.affine)defas_homogeneous(self):ifself.is_homogeneous:returnself.coordinatesreturnnp.hstack(self.coordinates,np.ones(...))defget_coords(self,homogeneous=False):"""Evaluate coordinates"""coords=self.affine @self.as_homogeneous()ifnothomogeneous:coords=coords[:, :-1]returncoords Then transforming can simply be: transformed=xfm @pointset (It takes a little bit more to get this actually to work.)fromdataclassesimportdataclass,replaceimportnumpyasnp@dataclassclassPointset:coordinates:np.ndarrayaffine:np.ndarrayis_homogeneous:booldef__rmatmul__(self,affine):"""Transform coordinates without evaluating"""assertis_affine(affine)returnreplace(self,affine=affine @self.affine)defas_homogeneous(self):ifself.is_homogeneous:returnself.coordinatesreturnnp.hstack(self.coordinates,np.ones((self.coordinates.shape[0],1)))defget_coords(self,homogeneous=False):"""Evaluate coordinates"""coords=self.affine @self.as_homogeneous()ifnothomogeneous:coords=coords[:, :-1]returncoords# Trick numpy into using our __rmatmul__ndim=2__array_priority__=99defis_affine(arr):returnarr.shape== (4,4)andnp.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 toaf4e08aCompare 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
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
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,GiftiDataArrayor FreeSurfer geometry arrays (which are just numpy arrays).We may also consider pulling
Parcelout of the CoordinateImage PR, for the sake of more efficiently handling subsets of coordinates.nibabel/nibabel/coordimage.py
Lines 147 to 168 in42e712e
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=SampledSpatialDataSampledSpatialDataalso includes the dimensionality of the points (generally 3)NDGrid=ImageGridImageGrid().ndindexcorresponds toNDGrid().get_coords('voxels')ImageGrid().ndcoordscorresponds toNDGrid().get_coords()ImageGridpreserves an original image header and pre-calculates an inverse affineImageGrid().ras()andImageGrid().index()apply the affine (or inverse) to passed indices/coordinates