ENH: Add support for GIFTI ExternalFileBinary #999
Conversation
ExternalFileName
Codecov Report
@@ Coverage Diff @@
## master #999 +/- ##
==========================================
+ Coverage 92.20% 92.22% +0.01%
==========================================
Files 100 100
Lines 12139 12164 +25
Branches 2121 2128 +7
==========================================
+ Hits 11193 11218 +25
Misses 618 618
Partials 328 328
Continue to review full report at Codecov.
|
|
Yes, sounds good to me. Especially if there are files in the wild, we should support them. I notice that the file is being opened relative to the cwd, not the gifti, which seems the more likely interpretation. |
|
Ahh, I hadn't thought of that - the CAT toolbox file which I have specifies the external file as relative to the GIFTI: |
|
That makes sense. Just need to update the code to reflect that. Right now it's an open can with no path adjustment. |
|
No problems - I'll follow up with that, and some tests as well - might find some more time this evening.. |
…, rather than all of its attributes as separate args. Also pass in name of file being parsed, in case data is to be loaded from an external file
|
Will add a bit more testing soon .. |
nibabel/gifti/parse_gifti_fast.py
Outdated
| with open(ext_fname, 'rb') as f: | ||
| f.seek(darray.ext_offset) | ||
| nbytes = np.prod(darray.dims) * dtype().itemsize | ||
| buff = f.read(nbytes) |
There was a problem hiding this comment.
This would certainly work, but if somebody's gone through the trouble of giving us a binary file for more efficient access, should we consider return a np.memmap or even an ArrayProxy so that the data is only loaded on-demand?
There was a problem hiding this comment.
Yes, good point - I had thought of that, but then just went with what is done for other <DataArray> types. I suppose this should be user-selectable as well - I'll add a mmap=True option to GiftiImage.from_filename and related ...
|
Ok, I've added mem-map support, and added a few extra test cases - ready for a review! |
This PR adds support for loading GIFTI data arrays from external files, using the
ExternalFileNameattribute of the<DataArray>element.I had a query from a user who is working with a SPM toolbox which generates GIFTI files where the data arrays are stored in external
.datfiles, and the<DataArray>element hasencoding="ExternalFileBinary". This led me to discover thatnibabeldoes not support GIFTI files which use this feature, presumably because the GIFTI spec is slightly under-specified regarding the expected format of the external file - from section 7.0 of the GIFTI spec:However, the CAT Toolbox simply stores its data as a raw uncompressed binary file, and I think that this is a sensible and reasonable interpretation of the above definition. If this assumption is made, then all of the necessary information (file offset, data type, row-/colum-major ordering, shape) will be available via the other attributes of the
<DataArray>element, so the required changes tonibabelare quite trivial.Happy to add in some unit tests and error checking if you think this is a useful contribution.