rust-ndarray
mapv_into_any doesn't respect storage type
Unlike mapv_into, mapv_into_any currently always returns a heap-allocated Array even if the source is something else - in my case, ArcArray.
It would be great if, like mapv_into, it was generic over storage types and allowed to clone ArcArray in-place when no copy is necessary.
cc @benkay86 who added it in the first place
I didn't think about ArcArray as a use case for mapv_into_any() when I contributed that PR, but I agree it would be nice to avoid unnecessary clones. @RReverser, would you mind providing a brief code snippet showing what you want to achieve? mapv_into_any() must return an owned array, but I think it might be possible to be generic over its return type to return an ArcArray instead of an Array.
Also, BTW, ndarray appears to be no longer actively maintained, see #1272. Some simple pull requests might still get merged, but it's unclear if or when a new release will happen.
mapv_into_any has the same limitations as map, map_mut, and mapv and mapv_into is the odd one (!)
It would be nice to solve but you sort of get into type system limitations, don't you?
@RReverser, would you mind providing a brief code snippet showing what you want to achieve?
Just want to do mapv_into_any to convert items on an ArcArray but reuse the existing ArcArray if it's already same type. Same as what it does on mapv_into.
mapv_into_any has the same limitations as map, map_mut, and mapv and mapv_into is the odd one (!)
Those methods don't take array by ownership so I don't think they're comparable.
It would be nice to solve but you sort of get into type system limitations, don't you?
Not in this case tbh. I think it can be generic over DataMut, just like mapv_into, so if ArcArray already has only one owner & is the same type, then it's modified in-place and in any other case it creates a new ArcArray.
Mapv_into_any does not use the same signature as mapv_into, because the element type changes. We get into the general Rust problem of translating "Foo<X>" into "Foo<Y>" (higher kinded types?). To us, they are both kinds of "Foo". But to Rust they are two unrelated and different types.
You might see that the incoming storage type is "S" in ArrayBase<S, ..> but the outgoing storage type will be something else S2 != S if the element types are not equal (A != B) because S depends on the element type.
If we look elsewhere in Rust, "Vec" is not a type, but "Vec<T>" for a particular T is a type.
We know how to do some array kind-preserving but element mapping operations, we have https://docs.rs/ndarray/latest/ndarray/struct.ArrayBase.html#method.assume_init which does this after all, but I think it's kind of awkward. Would be great to see what the solution would look like here.
To us, they are both kinds of "Foo". But to Rust they are two unrelated and different types.
Sure, but mapv_into_any already works around that by comparing TypeIds of the element. I don't see, however, how that restricts putting result storage repr to be the same as input - the implementation of mapv_into_any doesn't seem to restrict that in any way.
If you don't want my input on where challenges might lie - I've worked with these kinds of issues in ndarray before after all, when writing most of the crate - then maybe attempting an implementation and seeing what works in practice is a good way to go.
That... sounds unnecessarily hostile, but sure, I'm happy to give it a stab. I just thought @benkay86 might be more interested hence the issue.
higher kinded types?
Ah and yeah I see what you meant, but now that we have higher-kinder associated types I think K this might be still doable. If not, another type param could be just arbitrary storage destination.
The bigger problem is that one way or another this would be a breaking change at this point, as someone might rely on output being always Array, so I guess this would require a yet another separate function.
@bluss, happy to see you active on GitHub! Ndarray is a really valuable project, happy to see it hasn't been completely abandoned.
OK, I took a stab at this with #1283. Looks OK except for pre-existing issues with CI and clippy.
This is a breaking change that will require users to call mapv_into_any() differently. Currently mapv_into_any() always returns an Array, so there is no need for the caller to annotate the return type. #1283 makes mapv_into_any() generic over a return type of ArrayBase<T: DataOwned, _>, so the caller will need to be explicit about whether it should return an Array or an ArcArray. I don't think this is a big deal -- it's pretty similar to how Iterator::collect() works. But it may require a 0.16 release since it is a breaking change.
@benkay86 This is great, thanks! It's pretty much what I had in mind with making it generic over return type but I had the same concerns about backward compatibility so wanted to hear back on those first. Appreciate you implementing it though!
so the caller will need to be explicit about whether it should return an Array or an ArcArray
FWIW another option I was thinking about was just moving this method to specialised impls, that is, having individual methods mapv_into_any on Array, separate on ArcArray etc.
This would allow the caller to avoid specifying return type as a param, always returning same storage type as input, but would make it less flexible so not sure it's worth it.
I don't quite have the bandwidth. I'd like to bring in new people to maintain, if it can be done.