NF: volume computation function for nifti masks#952
Conversation
Compute volume of mask image. Equivalent to "fslstats /path/file.nii -V"
|
A few questions to help me guide this PR:
|
Codecov Report
@@ Coverage Diff @@
## master #952 +/- ##
==========================================
+ Coverage 91.84% 91.89% +0.04%
==========================================
Files 97 101 +4
Lines 12428 12548 +120
Branches 2191 2208 +17
==========================================
+ Hits 11415 11531 +116
- Misses 678 680 +2
- Partials 335 337 +2
Continue to review full report at Codecov.
|
|
I think you've forgot a
or
|
|
If we want to implement something equivalent to The command line should be in |
@0rC0 actually there is a return on line 248, it is just hidden by the codecov messages. return mask_volume_mm3 |
@effigies : Sounds good to me as well. But do you think every element of fslstats should be implemented with a single release or should we implement them gradually? |
|
I think gradually is fine. Let's call it Also, don't feel compelled to mimic the One thing to consider: |
|
Hello @JulianKlug, Thank you for updating! Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, Comment last updated at 2020-10-19 21:36:20 UTC |
- blank lines - variable declaration in example
|
There seems to be an issue where the example part of the docstring is interpreted as code and not as a string which then lets the tests fail. >>> path = 'path/to/nifti/mask.nii'
>>> img = nf.load(path) # path is contains a path to an example nifti mask
>>> mask_volume(img)Not quite sure how to resolve this... |
|
would be ready for #960 if the aforementioned issue is resolved |
effigies
left a comment
There was a problem hiding this comment.
Thanks for pushing on this. I've left a batch of comments. Feel free to argue and ask questions. :-)
nibabel/tests/test_imagestats.py
Outdated
| assert float(vol_mm3) == 2273328656.0 | ||
| assert float(vol_vox) == 284166082.0 |
There was a problem hiding this comment.
$ fslstats nibabel/tests/data/anatomical.nii -V
33825 270600.000000
I'm guessing (but haven't checked) the discrepancy is from values > 1.
There was a problem hiding this comment.
I thought so too, but hadn't time to check.
The volume computation is only reliable for 0-1 masks, but as I didn't find any in the sample data, I didn’t use one for the test.
bin/nib-stats.py
Outdated
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| main() |
There was a problem hiding this comment.
These files are actually out-of-date. Just add an entry to the entry points:
Lines 71 to 81 in 917afab
There was a problem hiding this comment.
Great hadn't seen that!
nibabel/imagestats.py
Outdated
| import numpy as np | ||
|
|
||
|
|
||
| def mask_volume(img, units='mm3'): |
There was a problem hiding this comment.
Thinking about this a bit, if units=="vox", then the output should be integer; there's no good reason to have a float for counting non-zero voxels. That makes the types a bit messy. What if we did (simplified):
def count_nonzero_voxels(img):
return np.count_nonzero(img.dataobj)
def mask_volume(img):
nz_vox = count_nonzero_voxels(img)
return np.prod(img.header.get_zooms()[:3]) * nz_voxThe conditional logic inside this function basically evaporates, and is moved into cmdline.stats.main to select which function to call.
There was a problem hiding this comment.
Sounds good to me. I have simply left the code a bit more verbose for better readability.
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
…eader.pixdim Co-authored-by: Chris Markiewicz <effigies@gmail.com>
…e in mm3 - use a mask-like volume for testing
effigies
left a comment
There was a problem hiding this comment.
Mostly style suggestions. And please use flake8 to check for any additional issues.
bin/nib-stats.py
Outdated
|
|
||
|
|
||
| if __name__ == '__main__': | ||
| main() |
nibabel/cmdline/tests/test_stats.py
Outdated
| class Capturing(list): | ||
| def __enter__(self): | ||
| self._stdout = sys.stdout | ||
| sys.stdout = self._stringio = StringIO() | ||
| return self | ||
| def __exit__(self, *args): | ||
| self.extend(self._stringio.getvalue().splitlines()) | ||
| del self._stringio # free up some memory | ||
| sys.stdout = self._stdout |
There was a problem hiding this comment.
thanks, didn't know about that one
nibabel/funcs.py
Outdated
| """ Utility function returning True if affine is nearly diagonal """ | ||
| rzs_aff = aff[:3, :3] | ||
| return np.allclose(rzs_aff, np.diag(np.diag(rzs_aff))) | ||
|
|
There was a problem hiding this comment.
We don't need to touch this file.
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
|
Sorry for the style issues. Should be fixed now. |
|
Thanks a lot! This is a good start to a handy utility, and I look forward to seeing it grow. |
Adds a function to compute the volume of a mask image in mm3.
Simple vanilla python equivalent to "fslstats /path/file.nii -V".
Reason: Useful for pure python projects where one can not rely on fsl.
To Do: