-
Notifications
You must be signed in to change notification settings - Fork 267
MincHeader.get_xyzt_units() #1098
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
Conversation
Hello @jennydaman, Thank you for updating!
Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 nibabel.
Comment last updated at 2022年04月01日 19:46:07 UTC
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@ ## master #1098 +/- ## ========================================== - Coverage 95.37% 95.27% -0.11% ========================================== Files 207 207 Lines 29652 29679 +27 Branches 3017 3021 +4 ========================================== - Hits 28282 28277 -5 - Misses 933 953 +20 - Partials 437 449 +12 ☔ View full report in Codecov by Sentry. |
Possibly convergent work: #567
@effigies PTAL or close?
Sorry, what does PTAL mean?
Please Take A Look (which you just did)
I assume you're seeing (in the CI output):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
nibabel/tests/test_image_api.py:234: in validate_data_interface
img = imaker()
nibabel/tests/test_image_api.py:608: in <lambda>
yield lambda: self.loader(img_params['fname']), img_params
nibabel/dataobj_images.py:504: in from_filename
return klass.from_file_map(file_map, mmap=mmap, keep_file_open=keep_file_open)
nibabel/minc1.py:353: in from_file_map
units = minc_file.get_units()
nibabel/minc1.py:117: in get_units
return tuple(d.units.decode('utf-8') for d in self._dims)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
.0 = <list_iterator object at 0x7f929c3f3700>
> return tuple(d.units.decode('utf-8') for d in self._dims)
E AttributeError: 'netcdf_variable' object has no attribute 'units'
Whoops. Looks like too much has changed in the past 2 years and this PR isn't going to work anymore.
Could we continue the conversation in #1116? If we decide get_xyzt_units is a desirable feature to have, I can look into how to support it for MINC.
The headers of MINC files have information which specifies the volume units (almost always mm). This PR exposes the data via
MincHeader.get_xyzt_units(), which is homologous toNifti1Header.get_xyzt_units(). But I don't work with 4D MINC so the 't' unit is simply"unknown".I'm thinking that it would be nice to have
get_xyzt_units()part of the header interface.This PR is kind of a draft? It works, though my intention is to open a discussion and perhaps support
get_xyzt_units()for other file formats too.