BF FreeSurfer nifti surfaces can have >3 dimensions

[effigies]

Quick workaround for https://mail.python.org/pipermail/neuroimaging/2015-July/000132.html.

This uses the FreeSurfer hacks on any shapes beginning with (-1, 1, 1) or (27307, 1, 6), which seems fairly safe, but we can restrict down to 4-dimensional, or possibly only allow (-1, 1, 1, 1) or (27307, 1, 6, 1) in the 4D case.

I guess we should also add tests for these cases.

[matthew-brett]

Seems reasonable. I'm afraid we do need some tests :)

[effigies]

Okay. Looking at https://code.google.com/p/fieldtrip/source/browse/trunk/external/freesurfer/save_nifti.m?r=8776#52 again, it looks like this MATLAB code at least only ever works with 4D volumes, but does allow the 4th dimension to vary. The tests I uploaded permit up to 7 dimensions, which is the max of hdr['dim'] capabilities, and verify the first three dimensions are either (27307, 1, 6) or (-1, 1, 1).

[bcipolli]

Looks reasonable to me as well. I would, however, add some comments to the get and set shape functions about the current limits of shape.

[effigies]

How about adding the following to set_data_shape?

    # For Nifti surface files, the first dimension is assumed to correspond
    # to vertices/nodes on a surface, and dimensions two and three are
    # assumed to have depth of 1. Dimensions 4-7 are not subject to
    # constraints beyond type bounds.

Or should I describe the hacks? I think the code in set_data_shape is about as readable as any comments I could add. I'm not sure what to add to get_data_shape that adds any clarity, except maybe (see set_data_shape for details).

[bcipolli]

@effigies that's just what I was looking for. I would put that on set_data_shape in the docstring, or in the Nifti1Image docstring if it's more widely applicable.

I found the code to be very readable, I doubt inline comments could help.

[effigies]

Added a variation on that comment. One possible quibble is that surface files that have <65536 nodes will not trigger constraints on dimensions 2 and 3, since they won't hit either of the large surface hacks. I'm not sure this is a problem, but I'd be open to suggestions of rewording.

[bcipolli]

@effigies I appreciate the note, and I'll follow up on this (to see if it is an issue) as I get closer to working on CIFTI/GIFTI stuff.

[matthew-brett]

When you say "Nifti surface files" - do you mean that this is part of the NIfTI standard somewhere? Or you just mean NIfTI as creatively reinterpreted by Freesurfer?

[bcipolli]

It's my understanding that CIFTI, which contains both surface and volume data, follows the NIFTI2 spec.

[matthew-brett]

Maybe add something to that effect here?

[effigies]

So, there are Nifti1 surface files (http://nifti.nimh.nih.gov/pub/dist/src/niftilib/nifti1.h):

#define NIFTI_INTENT_VECTOR    1007   /* for any other type of vector */

 /*! To signify that the vector value at each voxel is really a
     spatial coordinate (e.g., the vertices or nodes of a surface mesh):
       - dataset must have a 5th dimension
       - intent_code must be NIFTI_INTENT_POINTSET
       - dim[0] = 5
       - dim[1] = number of points
       - dim[2] = dim[3] = dim[4] = 1
       - dim[5] must be the dimensionality of space (e.g., 3 => 3D space).
       - intent_name may describe the object these points come from
         (e.g., "pial", "gray/white" , "EEG", "MEG").                   */

Annoyingly, this isn't how FreeSurfer actually implements its surfaces, presumably because of the need for large vector hacks.

[matthew-brett]

Excellent precision of the thing - could these go in the docstring, for posterity?

[effigies]

How about the following documentation output?

https://qnl-linux1.bu.edu/~effigies/nibabel/reference/nibabel.nifti1.html#nibabel.nifti1.Nifti1Header.set_data_shape

[matthew-brett]

Nice - but - can you clean up the appearance of the links in the output by using ReST links, defined at the bottom of the docstring?

[effigies]

Learning Sphinx, today. Have another look?

[matthew-brett]

Nice ! Does it look OK in the source code as well?

[matthew-brett]

Looks good apart from my line comments.

[effigies]

Pushed new commit. If source needs cleanup, let me know.

[matthew-brett]

Thanks for that - looks good now. Any more comments? Otherwise I will merge tomorrow.

[bcipolli]

@matthew-brett @effigies this seems ready to go?

[matthew-brett]

Thanks for reminding me.

[coveralls]

Coverage remained the same at 95.349% when pulling 0a3b0b8 on effigies:freesurfer4d into 60d4b72 on nipy:master.