Shortcut for the 10 AsPrimitive<T>

[nilgoyette]

You know that group of 10 AsPrimitive that we see quite often in nifti-rs (8 times) and probably in all other projects that use nifti-rs and generics?

u8: AsPrimitive<T>,
...
f64: AsPrimitive<T>,

I tried removed them, but of course I can't because it tries to read from all possible types and convert that to all possible types. This is an important requirement :)

Maybe this is more a question about Rust, but do you think that it's possible to create a "group" trait and use it everywhere? A kind of shortcut? I tested it

trait AllTypesToPrimitive<T>
where
    T: DataElement, // Added because it's used everywhere
    u8: AsPrimitive<T>,
    ...
    f64: AsPrimitive<T>,
{}

impl IntoNdArray for InMemNiftiVolume {
    fn into_ndarray<T>(self) -> Result<Array<T, IxDyn>>
    where T: AllTypesToPrimitive<T> { ... }

and tried using it but it seems that I don't know the right syntax or that's it's impossible. I get several errors that look like

^ the trait `typedef::_IMPL_NUM_FromPrimitive_FOR_NiftiType::_num_traits::AsPrimitive<T>` is not implemented for `f64`

It would be really nice to have this shortcut! Do you think it's possible?

[Enet4]

It's true that part of the reasons why the DataElement trait came to be was to aggregate these primitive conversion trait bounds. Perhaps there could be either an extension to this trait in order to support more use cases, or a dual trait for the opposite mapping.

Allow me to look deeper into this later this evening.

[nilgoyette]

Sorry, I don't want to be that kind of guy, asking the same question two times, but ... we want to use ndarray 0.13 at my job, so have you looked into this problem? Or if you don't have the time to do it, can you please do a release without a fix to this issue?

[Enet4]

Oh no, I had forgotten to take care of this one. :S My deepest apologies, I was clearly at fault here.

v0.9.0 is now released and published on crates.io. I ought to be more careful next time.

[nilgoyette]

Hey, we're all freely giving our time here, there's no compulsion! Thank you for the release!

[nilgoyette]

I wanted to finish this task before I ask you for yet another release, so I asked on users.rust-lang.org and they helped me reach a solution that I consider clean, but it requires nightly so I'll simply wait until trait_alias is stabilized. This could take years! AFAIK, there's no other way around it.

[Enet4]

I understand that the type alias could eventually be introduced without a breaking change, but it would still be unfortunate to have to depend on an unstable language feature.

Maybe an alternative would be constraining our data element to a new trait similar to NumCast (enables casting between any machine scalar), but as relaxed as AsPrimitive (does not return None if that value cannot be represented). That or we constrain it to NumCast and handle the edge cases at the implementation where necessary. This would like require a rewrite of our rescale transformation logic (IIRC this was the one that was designed to avoid precision loss), but it would rid us from all these trait bounds.

Please feel free to let me know if you'd like to try something like this and/or whether you need more assistance on this. I would be glad to see some progress on this issue.

[nilgoyette]

I don't want to depend on any unstable feature either. The way I see it, this task is a "nice to have", it simply makes the code more DRY. IMO, it doesn't deserve any big sacrifice of time or effort. That's why I wrote "I'll simply wait until trait_alias is stabilized."

If you believe your suggestion should be done anyway because it would make nifti-rs "better", then you should probably explain in more details what you have in mind. I'm not saying I'll code it asap but at least I'll be able to start when I have some free time at work.

[Enet4]

I figured that I would be probably better off to show this alternative. Please see #78 when you can.

Yes, this requires some repetition at this crate, but at least it doesn't spread so many trait bounds to our public APIs, which I believe is a fair price to pay.