-
Notifications
You must be signed in to change notification settings - Fork 16
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
Feat: Implement a visualisation feature to NiTransforms #212
base: master
Are you sure you want to change the base?
Conversation
Can you post screenshots? |
Creation of new directory vim containing scripts for plotting
…to plotting feature
…tion descriptions
…draft) interactive slider feature to switch between slices.
… plot. Implemented function descriptors.
…n up straneous comments and code; implement NotImplemented errors for sliders
Overview (3x3 plot grid): Distorted grid lines superimposed over deformed brain density map: Quiver plot of the deltas field: Hopefully this sets a good (or useful) base for the implementation of a visualisation module to nitransforms, or at least can act as a good starting point to spring new ideas. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #212 +/- ##
==========================================
+ Coverage 94.39% 94.71% +0.31%
==========================================
Files 15 16 +1
Lines 1713 2004 +291
Branches 323 374 +51
==========================================
+ Hits 1617 1898 +281
- Misses 79 83 +4
- Partials 17 23 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@effigies this is now ready for review! Latest plots can be accessed through the CircleCi workflow, under build_pytest > Artifacts. These shouldn't be too different (if at all) to the links posted above. |
…e or more spatial dimensions (see someones_displacement_field.nii.gz). Fixed.
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.
I'd be more comfortable with some of the numeric code if it came with tests; e.g., comparing the construction of the Jacobian determinant image with that generated by ANTs' CreateJacobianDeterminantImage
. Given that this is visualization, there is less concern that we have an error (that will presumably show up in a visualization) and no other numeric code depends on this.
I would be okay merging as-is, but I left some small comments.
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.
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.
I've worked more on the notebook and fixed this since the last commit, it simply requires a 3x3 subplot (I carelessly copy pasted a 1x3 figure)
Edit: committed to fix, should work now 👍
try: | ||
"""if field provided by path""" | ||
self._voxel_size = nb.load(transform).header.get_zooms()[:3] | ||
assert len(self._voxel_size) == 3 | ||
except TypeError: | ||
"""if field provided by numpy array (eg tests)""" | ||
deltas = [] | ||
for i in range(self._xfm.ndim): | ||
deltas.append((np.max(self._xfm._field[i]) - np.min(self._xfm._field[i])) | ||
/ len(self._xfm._field[i])) | ||
|
||
assert np.all(deltas == deltas[0]) | ||
assert len(deltas) == 3 | ||
self._voxel_size = deltas |
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.
It would be better not to rely on different code paths for tests and public API, if at all possible.
Can the test be rewritten to save a NIfTI image into tmp_path
and pass that to PlotDenseField()
?
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Addition of new file vis.py. Calls nitransform's nonlinear DenseFieldTransform class to illustrate corresponding transform objects through:
which can be illustrated all together (3x3 grid of plots, showing axial, coronal and sagittal projections for each graph) or individually.