search

kvark / mint

Math Interoperability Types
MIT License
256 stars 20 forks source link

make from_slice() method public #26

Closed not-fl3 closed 7 years ago

not-fl3 commented 7 years ago

Probably it was made hidden for a reason, but it's quite a usable method :)

My use case: transforming [f32; 4] arrays to Vector3 with Vector3::from_slice(&vector[0..3]).

not-fl3 commented 7 years ago

@Ralith any thoughts about this?

Ralith commented 7 years ago

I feel like what you really want here is to get a [f32; 3] from a [f32; 4] (a conversion mint has nothing to do with) and then use the already-available Into impl. While this is a bit tedious in current rust, const generics should allow a general solution to be built, making such an API in mint redundant. I'm a fan of mint having the smallest possible API to serve its purpose, which is why I didn't make this public to begin with.

kvark commented 7 years ago

While this is a bit tedious in current rust, const generics should allow a general solution to be built, making such an API in mint redundant.

Is it close on the release radar? I thought that Rust team is taking it slow to adopt those. If we are talking about, say, a year, then we shouldn't be concerned about extra API here. First, because we can always put #[deprecated] on it without breaking, and secondly because there might be other relevant changes in the language that would force us to release a minor/major version update anyway.

Now, I don't think it's going to be redundant since [f32; 4] -> [f32; 3] is not the only use case I can think of. Let's say you have a Vec<f32> that you read from somewhere and you want to extract some Vector3<f32> from it. Could do:

let data: Vec<f32> = ...;
let vectors = data.chunks(3).map(Vector3::from_slice);

Of course, it's not making anything more possible than it was, but the code is already there and it seems both useful for clients and safe. Would that reasoning be convincing enough, @Ralith ?

Ralith commented 7 years ago

Works for me. I didn't have a very strong opinion to start with, and the chunks example is particularly compelling.

Ralith commented 7 years ago

If it's to be public, would it make more sense to expose it as impl<'a, T> From<&'a [T]>?

not-fl3 commented 7 years ago

I am not exactly sure which way is better, but consider slices of the wrong size - maybe panic in From will be worse and less expected as panic in the method? Also, what do you think about two methods - with an assertion about slice length or making insufficient components as zeroes?

Ralith commented 7 years ago

I agree that a panicy From would be a bit surprising. I'm not strongly opposed to the status quo, just raising the question in case anyone had other thoughts.

I worry slightly that a zero-extending constructor is awfully purpose-specific. A more general alternative might be replacing from_slice with from_iter<I: IntoIterator<Item=T>>, allowing people to chain on whatever they want, though that's then more complex. I weakly favor only from_slice, but will happily defer to whatever @kvark prefers.

kvark commented 7 years ago

I don't think we should be doing anything more complex than from_slice. Thanks for all the input!