-
Notifications
You must be signed in to change notification settings - Fork 259
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
MincHeader.get_xyzt_units() #1098
Conversation
Hello @jennydaman, Thank you for updating! Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, 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):
|
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 |
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.