ndarray icon indicating copy to clipboard operation
ndarray copied to clipboard

Impl NdIndex<IxDyn> for &Vec<Ix> and Vec<Ix>

Open bderrett opened this issue 3 years ago • 2 comments
trafficstars

This change attempts to improve the ergonomics of indexing dynamic dimension arrays.

Let's say we have:

let mut a = ArrayD::<f32>::zeros(vec![1, 2]);
let index = vec![0, 0];

(Of course, in practice, the number of dims is dynamic.)

Then currently we can't do a[&index] = 1.0 or a[index] = 1.0. Instead we seem to need to do

let r: &[usize] = &index;
a[r] = 1.0;

This change makes the first two methods of indexing possible.

I'm new to ndarray, so there may be a discussion on this somewhere that I haven't seen. The new impls were created by copying the impl of &[Ix] (for &Vec<Ix>) and by modifying the impl of &[Ix] (for Vec<Ix>).

I can add tests for this if it looks useful?

bderrett avatar Jan 18 '22 08:01 bderrett

Thanks for the PR. I agree that it may be worth adding Index implementations for Vec<usize> and &Vec<usize>; there isn't much downside. Alternatively, we could add implementations for Vec<usize> and &T where T: NdIndex<IxDyn>.

@bluss What do you think?

Fwiw, this currently works with ndarray:

let mut a = ArrayD::<f32>::zeros(vec![1, 2]);
let index = vec![0, 0];
a[&*index] = 1.0;

(Note the extra * to use the Vec's Deref implementation.)

If index is &Vec<usize>, it's a bit more awkward: a[&**index] = 1.0 or a[index.as_slice()] = 1.0.

Regarding the implementation -- I'd suggest converting to &[usize] and using the existing NdIndex<IxDyn> for &'a [Ix] implementation. (Look at NdIndex<IxDyn> for &'a IxDyn for a similar case.)

jturner314 avatar Jan 22 '22 03:01 jturner314

Many thanks for the comments. The PR now adds implementations for Vec<usize> and &T where T: NdIndex<IxDyn>.

bderrett avatar Jan 23 '22 16:01 bderrett