ArrayInterface.jl
ArrayInterface.jl copied to clipboard
ismutable wrong for FillArrays
FillArrays are not mutable.
julia> ArrayInterface.ismutable(typeof(Fill(4, (2,2))))
true
I think that https://github.com/SciML/ArrayInterface.jl/blob/730592e19c5e3effb086a5cc0b61ecc1a1936a76/src/ArrayInterface.jl#L75-L81
Should be
function ismutable(::Type{T}) where {T<:AbstractArray}
if parent_type(T) <: T
return false # no parent, and have not hit a type we know about
else
return ismutable(parent_type(T))
end
end
ismutable{::Type{<:Array}} = true
ismutable{::Type{<:SparseVector}} = true
ismutable{::Type{<:SparseMatrixCSC}} = true
ismutable{::Type{<:BitArray}} = true
Alternatively
the fallback case could be: return T.mutable rather than false.
On the assumption that if you are a mutable struct that has subtypes AbstractArray you probably have defined setindex
I like the idea of mutability being something that needs to be opted into. I didn't originally make true the default, so I'm assuming that decision reflected that the majority of the ecosystem uses Array and parent_type wasn't a method at the time to dig into wrappers.
Does this break anything.
Does this break anything.
How many libraries are currently using ArrayInterface.jl?
It's easy enough to test a small handful locally, like DifferentialEquations.jl.
Barring breaking a bunch of stuff, I think it's okay to make false the default.
basically anything that is mutable will eventually parent it's way up a fundermentally mutable backing array type, probably Array.
Immutable things, like StaticArrays and FillArrays tend not to have parents in the first place
Should we suggest using ArrayInterface.can_setindex and ArrayInterface.can_change_size in documentation for most arrays since these methods are more specific?
No, can_setindex is different, for example, with GPUs.
I assume all AbstractFill are immutable? If that's true, we should just make an extension library for FillArrays that does that. I'll wait until someone who knows FillArrays.jl better responds and makes sure that's true.