Enet4 / nifti-rs

Rust implementation of the NIfTI-1 format
Apache License 2.0
42 stars 12 forks source link

Affine transformation #22

Closed nilgoyette closed 5 years ago

nilgoyette commented 6 years ago

We would like to contribute "affine" code to nifti-rs. A part of the code is in our private compagnie codebase and the rest is here.

I think it belongs in nifti-rs because:

In fact, I consider nifti-rs to be the Rust version of NiBabel, so of course it should be in nifti-rs. What's your opinion on this?

Disclaimer: Most of the code has been ported from NiBabel. I don't know much about licensing and I don't like to care about it.

Enet4 commented 6 years ago

That sounds quite interesting, and could narrow the gap towards other features of this library. :+1:

I consider nifti-rs to be the Rust version of NiBabel, so of course it should be in nifti-rs. What's your opinion on this?

To my understanding, NiBabel can handle a plethora of other neural imaging formats, whereas I wish to focus this library on NIfTI. We can add that logic anyway, and perhaps consider making it an opt-in or separate component if it becomes large enough to stand on its own.

Disclaimer: Most of the code has been ported from NiBabel. I don't know much about licensing and I don't like to care about it.

NiBabel is covered by the MIT license, so we can use it as long as we include their copyright + permission notices in the ported module .rs file.

fmorency commented 6 years ago

To my understanding, NiBabel can handle a plethora of other neural imaging formats, whereas I wish to focus this library on NIfTI. We can add that logic anyway, and perhaps consider making it an opt-in or separate component if it becomes large enough to stand on its own.

That's okay and it makes sense. Atm, we're only using NIFTI and we wrote our own library to handle TRK files 1 which are supported by the NiBabel streamline API. We don't plan to add/support other file formats in nifti-rs and will write new libraries/crates if required.

Enet4 commented 6 years ago

Indeed, that is fair and nice. :) Feel free to send in the PRs and I'll do the usual reviewing when possible.

nilgoyette commented 5 years ago

I started working on this and I might have some architecture and dependency questions.

I believe that the "natural" types for transformations and affines are from nalgebra, types like Vector3, Matrix3, Matrix4 and Quaternion

So, do we take nalgebra anyway and lock all the affine stuff behind a feature?

Enet4 commented 5 years ago

Yes, placing this behind an affine Cargo feature sounds like a reasonable approach for now. We can decide whether to make this the default later. The two array crates aren't necessarily a redundancy if they play different roles and allow us to gain access to the ecosystem's existing solutions.