nifti-rs icon indicating copy to clipboard operation
nifti-rs copied to clipboard

Support reading and writing of complex-valued data

Open twitzelbos opened this issue 2 years ago • 7 comments

I'm proposing (and volunteering) to add support for reading and writing of complex-valued data. I like to get buy-in here before proceeding.

twitzelbos avatar Jun 15 '23 18:06 twitzelbos

I confirm that no one else is doing it (afaik)

As you know, writing is already implemented for all standard Rust types. + RGB. This leads to code duplication which I really don't like. One of our goals was to merge write_nifti and write_rgb_nifti in order to have a single write_nifti function. As always, we forgot about it :)

I would prefer not having a write_complex_nifti function alongside two other writing functions, but I understand that playing with traits in Rust is much more complex and constrained than in C++. This being said, maybe it's super simple, maybe you simply need to add Complex to DataElement, I haven't checked.

nilgoyette avatar Jun 15 '23 19:06 nilgoyette

Okay, to summarize:

  • create a patch that adds the ability to write complex valued and RGB data to write_nifti
  • make sure reading works the same

If that sounds good, please assign the issue to me.

twitzelbos avatar Jun 15 '23 21:06 twitzelbos

I wrote about RGB because I don't like the current design, but you don't have to "fix" anything. If your goal is to handle complex number, you're free to do only that :)

nilgoyette avatar Jun 16 '23 01:06 nilgoyette

I made some progress, and I can successfully write and mostly read everything I want. A significant change I'm making is to remove safe_transmute in favor of bytemuck, which will simplify many things.

The reason is that safe_transmute and co. make use of custom traits to signal if something is safe to transmute. In Rust, you must own either the type or the trait to be allowed to implement a trait for a type, which keeps leading to dead ends. Bytemuck has sufficient adoption that many types implement its traits, getting us out of that hole.

twitzelbos avatar Jun 19 '23 19:06 twitzelbos

Since this is merged, can we push a new version of the crate to crates.io, please?

twitzelbos avatar Jul 05 '23 16:07 twitzelbos

Indeed, a new version is due. I will work on it when I'm able.

Enet4 avatar Jul 05 '23 16:07 Enet4

Done!

Enet4 avatar Jul 05 '23 20:07 Enet4