- Notifications
You must be signed in to change notification settings - Fork269
ENH: Add TriangularMesh data structure [BIAP9]#1257
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
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedSep 19, 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 #1257 +/- ##==========================================+ Coverage 92.19% 92.22% +0.02%========================================== Files 99 99 Lines 12458 12496 +38 Branches 2560 2565 +5 ==========================================+ Hits 11486 11524 +38 Misses 648 648 Partials 324 324
☔ View full report in Codecov by Sentry. |
c5458a3 to107ead6Compare| defadd_coordinates(self,name,coordinates): | ||
| self._coords[name]=coordinates |
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.
This is simple but perhaps a little clunky. We could also do:
| defadd_coordinates(self,name,coordinates): | |
| self._coords[name]=coordinates | |
| defadd_coordinates(self,*args,**kwargs): | |
| self._coords.update(dict(*args,**kwargs)) |
This would allow it to take any inputs that would generate adict, which might be a bit more ergonomic. I'm not 100% on this, so I haven't made the change.
…are shared after with_name()
#1251 Added
PointsetandGrid.#1090 also proposesTriangularMesh, but with the refactoring seen in#1251, that seemed like a bit of extra work for minimal benefit to the transformations use-case. This PR re-adds triangular meshes.Overall, the approach of multiple names feels clunky and pushes the concerns of surface families into pointsets.#1251 removed that, and this PR extends this by keeping
TriangularMeshfocused on the base data structure and movesTriangularMeshFamilylogic into aCoordinateFamilyMixinclass that permits swapping out coordinates with a.with_name()method (open to bikeshedding). Hence, we would change:to:
It also follows the lead of
Gridand gets rid of the omni-__init__in favor of targeted factory classmethods.Notes from previous discussion:
nibabel/nibabel/freesurfer/io.py
Lines 145 to 168 in590b226