Header fix

[nilgoyette]

There are several automatic fixes done on the header in NiBabel. I don't think it's relevant to have them all. IMO, fixing the magic number is wrong, but some may disagree.

I would add _chk_qfac because one Nifti I have crashes with nifiti-rs when I ask for the qform affine because pixdim[0] == 0.0. It works with NiBabel. I could fix it in all my outside tools, but I think it should be fixed here.

But maybe we're against all automatic fixes? What's your opinion on this?

[Enet4]

I would favor flexibility, in a similar way as the proposition in #49: it's OK that the library performs automatic header fixing, so long as it's either opt-in (disabled by default) or opt-out (enabled by default), and that such fixes are made clear in the documentation. It does seem to be a good use case for an Options pattern as well.

[nilgoyette]

Can you explain or give me a link explaining what's the Options pattern? I'm not totally sure what's you're talking about.

[Enet4]

Sure, it amounts to representing the options of an operation as a struct instead of function parameters. As the same "options" value is used to perform the operation, this would be an adapted form of the builder pattern.

The struct would be something like this:

struct ReaderOptions {
    /// whether to automatically fix value in the header
    fix_header: bool,
}

With the right implementation so that it can be used like this:

let obj = ReaderOptions::new()
    .fix_header(false)
    .open_file("my/file.nii.gz")?;

All methods are optional, and the method build can either consume or mutate the options depending on what they contain. The crate would still provide an easier open_file function that is equivalent to opening a file with the default options.

fn open_file<P>(path: P) -> Result<InMemNiftiObject>
where
    P: AsRef<Path>,
{
    ReaderOptions::new().open_file(path)
}

A good example in the std library is fs::OpenOptions.

[nilgoyette]

Ok, I thought that this ReaderOptions would simply be a structure to hold the options, that we give to a reader function/class to do the actual reading. But it's more like : this class does everything related to reading.

Ok, I'll code that. To avoid changing everything though, I'll simply call the existing tools when creating ReaderOptions so it should stay small.

[nilgoyette]

Ok, there are some things I'm unsure how to do. I can make it work, of course, but you probably have a vision on how it would look like if you wrote it. Will this ReaderOptions be the ONLY way to read an image, thus making all other stuff private?

I wrote this class in object.rs to have access to the private header and everything else that we may need later. But I see that we have two types of objects.

pub type InMemNiftiObject = GenericNiftiObject<InMemNiftiVolume>;
pub type StreamedNiftiObject<R> = GenericNiftiObject<StreamedNiftiVolume<R>>;

If ReaderOptions becomes the only way to read an image, I'm not sure how to handle this.

[Enet4]

This would not be the only way to construct the object, but it would ultimately be easier to use. The terminal option methods would then take care of passing the options to the implementation.

Regarding the existence of multiple object implementations, I believe this is best handled by using the same options struct while providing multiple terminal methods (pseudocode):

fn open_file<P>(self, path: P) -> InMemNiftiObject;

fn open_file_streamed<P>(self, path: P) -> StreamedNiftiObject<....>;

One other option would be creating independent option structs.

Please let me know if you need more assistance.

[Enet4]

Resolved with #70.