Add DenseArray <: AbstractStrided <: AbstractArray #50275
Labels
domain:arrays
[a, r, r, a, y, s]
Comments
Just do it, i.e., make a (draft) PR and we run pkgeval on it, and then we'll see. |
|
Ok, to be fair, one would have to document this. It probably makes sense to change Which still doesn't sound too hard, but is work. I'll be happy to do it if it stands a chance of being accepted. But I wouldn't want to waste everyone's time if it will be outright rejected, because this approach has been decided against from the start. BTW, I just tried cloning the repository and running |
|
Wait, I just noticed: SubArray will be strided only for strided parent arrays, right? So that part seems wrong. |
|
I was sloppy, my bad. I meant to say that the members of the Any idea how to get the tests to pass? I'm uncomfortable making changes when I have failing tests as it would be hard to know whether I'm making things worse or not. |
|
I'm not sure I follow. StridedSubArray subtypes SubArray, which subtypes AbstractArray. You can't "rewire" specific subtypes (depending on parameter types) to different supertypes. |
|
IIUC, this is now the trait |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The intent being "if you implement
strides, then you should<: AbstractStridedinstead ofAbstractArray.#2345 is 10 years old now. There still isn't a way to say "I have this function that requires that the array implements the
stridesmethod, even if it isn'tArrayorSubArray. There's still need for this.It seems to me the problem with #2345 is that it started with a very concrete small change, and devolved into a discussion of how to fix typing of arrays in Julia in general, which was not resolved.
I'm proposing we do not address this larger issue here. Just add a new
DenseArray <: AbstractStrided <: AbstractArray, and makeSubArray <: AbstractStrided.I'm not proposing deleting the current
StridedArrayunion, as this will break code. In contrast, this proposal doesn't break any existing code.It seems like a tiny change:
abstract type AbstractStrided{T,N} <: AbstractArray{T,N} end.abstract type DenseArray{T,N} <: AbstractArray{T,N} endtoabstract type DenseArray{T,N} <: AbstractStridedT,N} end.struct SubArray{T,N,P,I,L} <: AbstractArray{T,N}tostruct SubArray{T,N,P,I,L} <: AbstractStrided{T,N}.That's it.
It would have been nice if this was done instead of creating the original
StridedArrayunion, but that's in the past. SoStridedArrayunion becomes "the strided version of [Sub]Array. There's value in that.Just adding this will not magically update existing packages that today either require
DenseArraywhich is an overkill (likeStrided.jl), or just give up and useAbstractArray(likePythonCall.jl). But once this is in the standard library, such packages will eventually start using it.Is there any downside? Why not "just do it"?
The text was updated successfully, but these errors were encountered: