Support NIFTI-2, resolves #89

[benkay86]

Adds support for the NIFTI-2 header format. The PR is just enough to compile and run tests. If approved, documentation and additional test cases will be needed in a subsequent PR. These changes are not backwards compatible.

• Types and methods outside the header have been changed to default to the larger NIFTI-2 types. For example, nifti::volume::shape::Dim is now based off u64 instead of u16.

• NiftiHeader is changed to an enum that contains either a Nifti1Header or Nifti2Header structure. NiftiHeader implements getter/setter methods to modify the underlying header without having to worry about which version it is. The underlying structure can also be accessed directly.

let mut hdr = NiftiHeader::default();
hdr.slice_duration = 1.0; // no longer works
hdr.set_slice_duration(1.0); // OK
let slice_duration: f32 = hdr.get_slice_duration(); // no longer works
let slice_duration: f64 = hdr.get_slice_duration(); // OK
// Conversion to larger NIFTI-2 types always succeeds.
let mut hdr2: Nifti2Header = hdr.into_nifti2();
hdr2.slice_duration = 1.0; // OK
let slice_duration: f64 = hdr2.slice_duration; // OK
// Conversion to smaller NIFTI-1 types is fallible.
let mut hdr1: Nifti1Header = hdr2.into_nifti().into_nifti1().unwrap();
hdr1.slice_duration = 1.0; // OK
let slice_duration: f32 = hdr1.slice_duration; // OK

Migration should otherwise be relatively painless.

• NiftiHeader::from_file() and from_reader() automatically detect the header version in the given file or input stream.
• WriterOptions::write_nifti() automatically writes out whichever header version is referenced by the WriterOptions.
• NiftiHeader::into_nifti1() and into_nifti2() conveniently convert between header versions.

[liningpan]

Hi, NIFTI2 support would be very helpful!

Can we add additional intent codes from the HCP pipeline?

https://github.com/Washington-University/workbench/blob/master/src/FilesBase/nifti2.h#L33-L49

const int32_t NIFTI_INTENT_CONNECTIVITY_UNKNOWN=3000;
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE=3001;
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE_TIME=3002;//CIFTI-1 name
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE_SERIES=3002;//CIFTI-2 name
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED=3003;
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED_TIME=3004;//ditto
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED_SERIES=3004;
const int32_t NIFTI_INTENT_CONNECTIVITY_CONNECTIVITY_TRAJECTORY=3005;//ditto
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE_TRAJECTORY=3005;
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE_SCALARS=3006;
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE_LABELS=3007;
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED_SCALAR=3008;
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED_DENSE=3009;
const int32_t NIFTI_INTENT_CONNECTIVITY_DENSE_PARCELLATED=3010;
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED_PARCELLATED_SERIES=3011;
const int32_t NIFTI_INTENT_CONNECTIVITY_PARCELLATED_PARCELLATED_SCALAR=3012;

However, several variables map to the same number which is not possible for rust enum.

[benkay86]

Apologies, I had a baby and kind of lost track of this! It looks like there is still broad interest in NIFTI-2 support, and aside from some formatting issues and the need for more test cases, the biggest obstacle is what to name getter methods.

@nilgoyette is right that Rust eschews getter methods in general, preferring the simplicity and explicitness of accessing a struct's field directly. The current PR allows this by resolving the NiftiHeader enum to a Nifti1Header or Nifti2Header struct that can be manipulated directly. In addition to this, convenient getter and setter methods which are agnostic to the underlying header version are provided for automatic conversion between field types.

In such cases where getter/setter methods are needed, the Rust convention is to name the setter for the field foo set_foo() and simply name the getter foo(). However, there is a problem for nifti-rs because a few fields already have getter-like functions with the same name. For example, NiftiHeader::dim is a [u16;8] and NiftiHeader::dim() returns a validated slice of this field. What should we name a getter that returns the raw field?

Possibilities:

• Name raw getters get_foo(). While this is verbose and goes against Rust convention, it is easily understood and avoids name conflicts with existing methods.
• Rename the existing methods something else like foo_validated() and name the raw getter foo(). This is more idiomatic but breaks the API. (Maybe not such a big deal since the PR contains other breaking changes in corner cases.)
• Don't provide a raw getter for fields that require some kind of validation or parsing. Make the user cast the enum down to a Nifti1Header or Nifti2Header if raw access to the field is desired.

Thoughts?

[liningpan]

I also tried to implement nifti-2 support myself. I took a more aggressive approach and cast nifti-1 and nifti-2 header to a common type derived from nifti-2. The common type also exposes Enum types directly instead of through a getter. I believe connectome workbench took a similar approach. However, this will complete nuke the current interface.

I also took a few test cases from nibabel, for nifti-2 unit tests.

You can take a look my experimental branch here if you are interested. https://github.com/liningpan/nifti-rs/tree/nifti2

[Enet4]

However, there is a problem for nifti-rs because a few fields already have getter-like functions with the same name. For example, NiftiHeader::dim is a [u16;8] and NiftiHeader::dim() returns a validated slice of this field. What should we name a getter that returns the raw field?

I don't take issue in introducing a breaking change to the API, especially for introducing such a big feature to the crate. raw_dim would make a decent method name for the slice as saved in the header (thus only available behind specific version headers).

Aside from this, I would also like to ensure that 1) it stays possible to work with a specific version explicitly, and that 2) calculations on NIfTI-1 can still use f32 as common ground, as mentioned in this comment.