Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
effigies merged 10 commits intonipy:masterfromeffigies:enh/pointset
Sep 19, 2023

Conversation

@effigies
Copy link
Member

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 asSpatialImage,GiftiDataArray or FreeSurfer geometry arrays (which are just numpy arrays).

We may also consider pullingParcel out of the CoordinateImage PR, for the sake of more efficiently handling subsets of coordinates.

classParcel:
"""
Attributes
----------
name : str
structure : ``Pointset``
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):
returnf'<Parcel{self.name}({len(self.indices)})>'
def__len__(self):
returnlen(self.indices)
def__getitem__(self,slicer):
returnself.__class__(self.name,self.structure,self.indices[slicer])

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 affine
    • ImageGrid().ras() andImageGrid().index() apply the affine (or inverse) to passed indices/coordinates

@codecov
Copy link

codecovbot commentedSep 7, 2023
edited
Loading

Codecov Report

Patch coverage:98.78% and project coverage change:+0.04% 🎉

Comparison is base(5f37398) 92.16% compared to head(5ded851) 92.21%.
Report is 7 commits behind head on master.

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
Files ChangedCoverage Δ
nibabel/pointset.py98.78% <98.78%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

Copy link
Contributor

@oestebanoesteban left a 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

@effigies
Copy link
MemberAuthor

@oesteban What do you think about adding anaffine directly toPointset?

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__matmul__, so a final call toget_coords() would be necessary.

@effigieseffigiesforce-pushed theenh/pointset branch 2 times, most recently from2607f17 toaf4e08aCompareSeptember 18, 2023 20:06
@pep8speaks

This comment was marked as outdated.

@effigies
Copy link
MemberAuthor

I've gone ahead and pushed forward on bundling affines withPointsets, defaulting to identity.Grid is now a simple wrapper that provides factory functions for creating aPointset from a spatial image (all coordinates) or a mask (selected coordinates only).

Updated comparisons toImageGrid:

  • ImageGrid().ndindex corresponds toNDGrid().coordinates (untransformed)
  • ImageGrid().ndcoords corresponds toNDGrid().get_coords() (transformed)

LMK what you think. It can be reverted, if this doesn't make sense to anybody else.

Copy link
Contributor

@oestebanoesteban left a 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

@effigies
Copy link
MemberAuthor

I've removed the triangular meshes from this for now. The existing architecture depended too much on the previousPointset implementation, and I don't want to hold this up.

@effigieseffigies merged commit590b226 intonipy:masterSep 19, 2023
@effigieseffigies deleted the enh/pointset branchSeptember 19, 2023 18:54
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@oestebanoestebanoesteban approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@effigies@pep8speaks@oesteban

[8]ページ先頭

©2009-2025 Movatter.jp