Conversation
ararslan
added
arrays
|
Could you show an example of how this can be used to address JuliaStats/NullableArrays.jl#167? |
|
Haven't tested yet, but something like this should work: promote_type_cat(::Type{S}, ::Type{T}) where {S<:AbstractArray, T<:NullableArray} =
NullableArray{promote_type(eltype(S), eltype(T))}
promote_type_cat(::Type{S}, ::Type{T}) where {S<:NullableArray, T<:AbstractArray} =
NullableArray{promote_type(eltype(S), eltype(T))}
similar{T,A<:NullableArray{T},N}(::Type{A}, dims::Dims{N}) = NullableArray{T, N}(dims) |
|
This is very interesting @nalimilan. From your example, it seems that The final thing is - if we need a specific promotion mechanism for concatenation - do we in general need a promotion mechanism for |
That's just for simplicity for a first approach. I've listed it in the TODOs (when I mentioned
That's an interesting question. Currently the fallbacks for |
|
For triage: This clearly won't make the feature freeze, but in the perspective of introducing this mechanism in the 1.x series I wonder whether we shouldn't change the rule that the type of the first array is used. Indeed, it would not fit very well in a general promotion mechanism: it's as if Maybe it would be more appropriate to always return an |
|
That might be a bit less breaking but not by much. Triage would rather punt on this for 1.0. |
|
FWIW, this would be really useful for operator-overloading-based AD systems like Flux and Knet currently have. It's current hard for us to intercept calls like |
|
Interestingly, dplyr is developing a system for concatenation which is very similar to this PR: https://stat.ethz.ch/pipermail/r-devel/2018-August/076550.html For this to work, they even have to introduce a promotion mechanism (which doesn't exist in R). |
This used to make a lot of references to design issues with the SparseArrays package (JuliaLang/julia#2326 / JuliaLang/julia#20815), which result in a non-sensical dispatch arrangement, and contribute to a slow loading experience do to the nonsense Unions that must be checked by subtyping.
This used to make a lot of references to design issues with the SparseArrays package (JuliaLang/julia#2326 / JuliaLang/julia#20815), which result in a non-sensical dispatch arrangement, and contribute to a slow loading experience do to the nonsense Unions that must be checked by subtyping. (cherry picked from commit 5a922fadfef083b83d983ac84f49a4460baa42ab) (cherry picked from commit 8fd5f279c8c81a8e59dec3852235c7dd5c88b143)
|
@nalimilan Are you still interested in this PR? |
|
This is blocked by lack of input from core devs AFAICT. |
7 participants
This is a first attempt to explore a fix for #2326, i.e. stop relying on the type of the first array to decide the type of the result of
catfunctions. The idea is to introduce a promotion system similar topromotebut for concatenation.Remarks/issues to resolve:
promotemechanism, but it turns out we need a separate one. For example, concatenatingDiagonalmatrices should give a sparse matrix, not aDiagonalmatrix. IOW, promoting a series of identical types for concatenation is not the same as promoting them for conversion (which is the goal ofpromote).promote_eltype) and the container type. That could allow e.g. returning aBitArraywhen concatenating anArray{Bool} with aBitArray`. The number of dimensions isn't involved since it depends on the kind of concatenation performed in a way that would make the system too complex.Arrayobjects unless a promotion rule has been defined. This could be changed so that concatenations involving only arrays of a custom type default to that type (though I'm not sure it's easy to write using dispatch). Thoughts? This is also an issue due to the next bullet.similar(::Type{A{T}}, dims)methods being defined for all types which define promotion rules, i.e. which want to opt-out from concatenation returning standardArrayobjects. AFAICT this is required since there is no generic method to take an array type and change just its element type (cf. this discussion aboutfixate_eltype); indeed that's not possible e.g. forBitArray. I think that method could be made one of the fundamental requirements ofAbstractArray, possibly redefining othersimilarmethods to call it by default: with an instance, you also have the type, but the contrary doesn't work.promote_ruleto avoid the need to define methods twice for each ordering of arguments.typed_*catlowering toeltyped_*catas it was simpler to prevent confusion. A less disruptive solution would be to preserve the existing lowering, and use another name for the new methods which take the array type instead of the element type.promote_type_catname should probably be improved.