-
Notifications
You must be signed in to change notification settings - Fork 1
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
Specification draft #1
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Satrajit Ghosh <[email protected]>
I have moved the python implementation to its own branch (PR #2), so that this PR is only concerned with the specification. |
inviting @effigies and @yarikoptic for comments. there are two other PRs open for the reference implementations in python and julia. |
| `complex128` | `1792` | `"c16"` | | ||
| `complex256` | `2048` | `"c32"` | | ||
| `rgba32` | `2304` | <code>[["r", "|u1"], ["g", "|u1"], ["b", "|u1"], ["a", "|u1"]]</code> | | ||
| `bool` | 🛑 unsupported! | <code>"|b1"</code> | |
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.
via nifti1.h
#define DT_BINARY 1 /* binary (1 bit/voxel) */
| `bool` | 🛑 unsupported! | <code>"|b1"</code> | | |
| `bool` | `1` | <code>"|b1"</code> | |
Not to say it's not a terrible idea to try to use it...
The nifti header ([v1](https://nifti.nimh.nih.gov/pub/dist/src/niftilib/nifti1.h) | ||
or [v2](https://nifti.nimh.nih.gov/pub/dist/doc/nifti2.h)) **MUST** be encoded as | ||
a string using [base64](https://en.wikipedia.org/wiki/Base64) encoding and saved in | ||
the group-level `.zattrs` under the `["nifti"]["base64"]` key: |
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.
As suggested in bids-standard/bids-specification#1704, I would consider saving the header as a binary array inside the base group instead of a JSON metadata field inside zattrs
. A NIfTI-1 header could be a scalar array of type |S348
(np.array(img.header.binaryblock)
) or an array of bytes 348 long (np.frombuffer(img.header.binaryblock, dtype='u1')
).
Do you have any thoughts on handling NIfTI extensions?
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.
part of the challenge is the interplay between ome-zarr and nifti-zarr. as long as we avoid those we should be ok. perhaps just call the key niftiheader
. i'm sure @balbasty can come up with some specific form that satisfies both constraints :)
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 like saving the binary block as an array! We'd indeed have to check that it does not violate OME validation.
For extensions, we could allow the binary block to contain everything up to the image data. It would mean that the header can be 348 or 352 or 352 + extension_size bytes. In that case, probably better to have the array be of type u1
than S348
. We should also probably require that no compression be used for this array.
Do we have to store it in nifti/0/
or could it go directly in nifti/
?
Would we still keep a JSON version of the header in .zattrs
?
Do we want to formalize the conversion of nifti extensions to JSON?
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 don't see anything in the OME-Zarr spec prohibiting additional named arrays, but I have not played with OME-Zarr tooling to see if it would ignore such an array.
I think it matters very little whether you use |S348
or u1
for your dtype, as the |S<N>
is automatically calculated from np.array(binaryblock)
.
For extensions, including them directly in the header seems fine to me, and consistent with treating nifti/0
as a .hdr
file. I wasn't sure if you might want to do something a little more involved and split them, e.g.,
nifti/
.zgroup
header/
0
.zarray
extensions/
.zgroup
0/
0
.zarray
[...]
n/
0
.zarray
This could have utility for manipulating individual extensions (for example, the AFNI extension has an appendable log) without forcing others to be shifted. But headers are the cheap part of the file, so this probably isn't worth worrying about.
Do we have to store it in
nifti/0/
or could it go directly innifti/
?
From the API, you can use /nifti
, but zarr will still need to create a chunk file 0
, e.g.:
img.nii.zarr
├── 0
│ ├── 0.0.0
│ ├── [...]
│ ├── 3.3.3
│ └── .zarray
├── nifti
│ ├── 0
│ └── .zarray
└── .zgroup
Would we still keep a JSON version of the header in
.zattrs
?
I would personally prefer not to. A single source of truth that everybody already knows how to parse seems preferable to a JSON source that needs to be validated as close enough to the header.
The discussion of extensions makes me think you may want to consider versioning nifti-zarr itself and recording the version in the root .zattrs. I don't think it would be a good use of time to try to create a future proof layout now, but if it ever becomes worth revisiting, a version number that could be incremented would be nice. |
I've implemented the new specification that uses a "nifti" zarr-array in new "0.2" branches. See differences between 0.1 and 0.2 here:
I've also checked that nifti-zarr files are valid ome-zarr files using their online validator. I am not sure what's the best way to proceed, open a new PR? I think the remaining questions are:
Edit: The julia implementation has now been updated. Note that neither the python nor julia implementation currently handle extensions. |
I guess you mean schema of the ".nifti" within
a job for validator. It is unfortunate that AFAIK there is no yet universal support for extending zarr validators... FWIW I am chiming in at which might be of interest (even if to bring my idea down ;) ). |
Yes, although I am also thinking of @effigies' idea of splitting out header extensions into multiple arrays under a nifti group, which could be added in a potential v2, if the community felt like it. |
@yarikoptic mentioned this thread to me. I just want to point out that JSON wrapper of NIFTI has already existed via my JNIfTI spec since 2019, and has been extensively used in our NoSQL database based data portal NeuroJSON.io for in-browser rendering, for example https://neurojson.org/db/abide2/ABIDEII-BNI_1#highlight=NIFTIHeader you could also click on any of the purple-colored links in the "External data" section of each dataset to render JNIFTI data volumes in browser, for example https://neurojson.org/db/adhd200/Brown lightweight parsers in matlab/octave/python/javascript for reading/writing jnifti files already exist, and it support multiple compression codecs, such as the blosc2 codecs I mentioned in bids-standard/bids-specification#1614 (comment). We have a poster about JNIFTI in OHBM 2022 Data grouping via at least for the NIFTI header metadata part, it would be nice not duplicating efforts - converting between different metadata conventions can really be frustrating to developers. |
Thanks @yarikoptic for connecting, and thanks @fangq for reaching out! It does absolutely make sense to use the jnifti spec for the json version of the header. I guess the most straighforward way of doing this would be to make nifti/.zattrs a valid jnifti file (without the "NIFTIData" key). Have you had a look at the rest of the spec? Your input would be appreciated. I would imagine you'd be a proponent of getting rid of the binary header altogether and use a jnifti only header? Our initial idea was that shipping the binary header required even less work for developers, who most likely already implement a parser for it. |
that would be fantastic!
I will take a deeper look. one think I noticed is on the dimension orders in jnifti's I understand that you only going to need please also take a look at my spec and see anything you expect discrepancies.
regarding the binary header, in my matlab implementation, I am currently using the following two functions to dynamically convert jnifti header to a valid nifti-1/2 header https://github.com/NeuroJSON/jnifty/blob/master/jnii2nii.m (and the reversed direction is done in https://github.com/NeuroJSON/jnifty/blob/master/niiheader2jnii.m) this way, we don't need to maintain two buffers with duplicated information, which is a risk for discrepancy - for example, if someone modified the jnifti header, the binary buffer must also be synchronized to be consistent if you have the info in two places. I am in the process of translating my matlab functions to Python, happy to share once it is done. also feel free to port if it is easier for you to do it. |
Work-in-progress