coverage and clean up (#79)

* Tests and cleanup

* Move things in correct places

* Switch to _promote_typejoin

* typo

Author: Pietro Vertechi

README.md

@@ -182,7 +182,7 @@ julia> map(t -> t.b ^ t.a, LazyRows(t))
 
 ## Advanced: structures with non-standard data layout
 
-StructArrays support structures with non-standard data layout (where `getproperty` has been overloaded or where the constructors are non-standard). The user is required to provide an overloaded `staticscheme` for their type (to give the names and types of the properties of a given type) as well as a `createinstance` method. Here is an example of a type `MyType` that has as properties either its field `data` or properties of its field `rest` (which is a named tuple):
+StructArrays support structures with non-standard data layout (where `getproperty` has been overloaded or where the constructors are non-standard). The user is required to provide an overloaded `staticschema` for their type (to give the names and types of the properties of a given type) as well as a `createinstance` method. Here is an example of a type `MyType` that has as properties either its field `data` or properties of its field `rest` (which is a named tuple):
 
 ```julia
 using StructArrays

src/collect.jl

@@ -109,11 +109,6 @@ function grow_to_structarray!(dest::AbstractArray, itr, elem = iterate(itr))
     return dest
 end
 
-function to_structarray(::Type{T}, nt::C) where {T, C<:Tup}
-    S = add_params(T, eltypes(astuple(C)))
-    return StructArray{S}(nt)
-end
-
 widenstructarray(dest::AbstractArray, i, el::S) where {S} = widenstructarray(dest, i, S)
 
 function widenstructarray(dest::StructArray{T}, i, ::Type{S}) where {T, S}
@@ -123,7 +118,8 @@ function widenstructarray(dest::StructArray{T}, i, ::Type{S}) where {T, S}
     cols = fieldarrays(dest)
     if names == propertynames(cols)
         nt = map((a, b) -> widenstructarray(a, i, b), cols, strip_params(sch)(types))
-        return to_structarray(T, nt)
+        ST = _promote_typejoin(S, T)
+        return StructArray{ST}(nt)
     else
         return widenarray(dest, i, S)
     end

src/interface.jl

@@ -9,12 +9,5 @@ end
 
 staticschema(::Type{T}) where {T<:Tup} = T
 
-astuple(::Type{NamedTuple{names, types}}) where {names, types} = types
-astuple(::Type{T}) where {T<:Tuple} = T
-
-function fields(::Type{T}) where {T}
-    fieldnames(staticschema(T))
-end
-
-strip_params(::Type{<:Tuple}) = Tuple
-strip_params(::Type{<:NamedTuple{names}}) where {names} = NamedTuple{names}
+createinstance(::Type{T}, args...) where {T} = T(args...)
+createinstance(::Type{T}, args...) where {T<:Union{Tuple, NamedTuple}} = T(args)

src/lazy.jl

@@ -24,8 +24,7 @@ end
 staticschema(::Type{<:LazyRow{T}}) where {T} = staticschema(T)
 buildfromschema(f, ::Type{<:LazyRow{T}}) where {T} = buildfromschema(f, T)
 
-iscompatible(::Type{<:LazyRow{S}}, ::Type{StructArray{T, N, C}}) where {S, T, N, C} =
-    iscompatible(S, StructArray{T, N, C})
+iscompatible(::Type{<:LazyRow{R}}, ::Type{S}) where {R, S<:StructArray} = iscompatible(R, S)
 
 (s::ArrayInitializer)(::Type{<:LazyRow{T}}, d) where {T} = buildfromschema(typ -> s(typ, d), T)
 

src/structarray.jl

@@ -81,7 +81,7 @@ Base.convert(::Type{StructArray}, v::StructArray) = v
 Base.convert(::Type{StructVector}, v::AbstractVector) = StructVector(v)
 Base.convert(::Type{StructVector}, v::StructVector) = v
 
-function Base.similar(::Type{StructArray{T, N, C}}, sz::Dims) where {T, N, C}
+function Base.similar(::Type{<:StructArray{T, <:Any, C}}, sz::Dims) where {T, C}
     buildfromschema(typ -> similar(typ, sz), T, C)
 end
 
@@ -171,12 +171,6 @@ function Base.copyto!(I::StructArray, doffs::Integer, J::StructArray, soffs::Int
     return I
 end
 
-function Base.cat(args::StructArray...; dims)
-    f = key -> cat((getproperty(t, key) for t in args)...; dims=dims)
-    T = mapreduce(eltype, promote_type, args)
-    StructArray{T}(map(f, fields(eltype(args[1]))))
-end
-
 function Base.resize!(s::StructArray, i::Integer)
     for a in fieldarrays(s)
         resize!(a, i)
@@ -188,12 +182,12 @@ function Base.empty!(s::StructArray)
     foreachfield(empty!, s)
 end
 
-for op in [:hcat, :vcat]
+for op in [:cat, :hcat, :vcat]
     @eval begin
-        function Base.$op(args::StructArray...)
-            f = key -> $op((getproperty(t, key) for t in args)...)
+        function Base.$op(args::StructArray...; kwargs...)
+            f = key -> $op((getproperty(t, key) for t in args)...; kwargs...)
             T = mapreduce(eltype, promote_type, args)
-            StructArray{T}(map(f, fields(eltype(args[1]))))
+            StructArray{T}(map(f, propertynames(args[1])))
         end
     end
 end

src/utils.jl

@@ -46,21 +46,13 @@ end
 
 foreachfield(f, x::T, xs...) where {T} = foreachfield(staticschema(T), f, x, xs...)
 
-createinstance(::Type{T}, args...) where {T} = T(args...)
-createinstance(::Type{T}, args...) where {T<:Union{Tuple, NamedTuple}} = T(args)
-
-add_params(::Type{T}, ::Type{C}) where {T, C<:Tuple} = T
-add_params(::Type{T}, ::Type{C}) where {T<:Tuple, C<:Tuple} = C
-add_params(::Type{<:NamedTuple{names}}, ::Type{C}) where {names, C<:Tuple} = NamedTuple{names, C}
-add_params(::Type{<:Pair}, ::Type{Tuple{S, T}}) where {S, T} = Pair{S, T}
-
 """
 `iscompatible(::Type{S}, ::Type{V}) where {S, V<:AbstractArray}`
 
 Check whether element type `S` can be pushed to a container of type `V`.
 """
 iscompatible(::Type{S}, ::Type{<:AbstractArray{T}}) where {S, T} = S<:T
-iscompatible(::Type{S}, ::Type{StructArray{T, N, C}}) where {S, T, N, C} = iscompatible(astuple(staticschema(S)), astuple(C))
+iscompatible(::Type{S}, ::Type{<:StructArray{<:Any, <:Any, C}}) where {S, C} = iscompatible(astuple(staticschema(S)), astuple(C))
 
 iscompatible(::Type{Tuple{}}, ::Type{T}) where {T<:Tuple} = false
 iscompatible(::Type{T}, ::Type{Tuple{}}) where {T<:Tuple} = false
@@ -72,6 +64,26 @@ end
 
 iscompatible(::S, ::T) where {S, T<:AbstractArray} = iscompatible(S, T)
 
+function _promote_typejoin(::Type{S}, ::Type{T}) where {S<:NTuple{N, Any}, T<:NTuple{N, Any}} where N
+    head = _promote_typejoin(Base.tuple_type_head(S), Base.tuple_type_head(T))
+    tail = _promote_typejoin(Base.tuple_type_tail(S), Base.tuple_type_tail(T))
+    return Base.tuple_type_cons(head, tail)
+end
+
+_promote_typejoin(::Type{Tuple{}}, ::Type{Tuple{}}) = Tuple{}
+function _promote_typejoin(::Type{NamedTuple{names, types}}, ::Type{NamedTuple{names, types′}}) where {names, types, types′}
+    T = _promote_typejoin(types, types′)
+    return NamedTuple{names, T}
+end
+
+_promote_typejoin(::Type{S}, ::Type{T}) where {S, T} = Base.promote_typejoin(S, T)
+
+function _promote_typejoin(::Type{Pair{A, B}}, ::Type{Pair{A′, B′}}) where {A, A′, B, B′}
+    C = _promote_typejoin(A, A′)
+    D = _promote_typejoin(B, B′)
+    return Pair{C, D}
+end
+
 function replace_storage(f, v::AbstractArray{T, N})::AbstractArray{T, N} where {T, N}
     f(v)
 end
@@ -106,9 +118,15 @@ function replace_storage(f, s::StructArray{T}) where T
     StructArray{T}(newcols)
 end
 
-to_tup(c::T) where {T} = to_tup(c, fields(T))
+to_tup(c::T) where {T} = to_tup(c, fieldnames(staticschema(T)))
 function to_tup(c, fields::NTuple{N, Symbol}) where N
     t = ntuple(i -> getproperty(c, fields[i]), N)
     return NamedTuple{fields}(t)
 end
 to_tup(c, fields::NTuple{N, Int}) where {N} = ntuple(i -> _getproperty(c, fields[i]), N)
+
+astuple(::Type{NamedTuple{names, types}}) where {names, types} = types
+astuple(::Type{T}) where {T<:Tuple} = T
+
+strip_params(::Type{<:Tuple}) = Tuple
+strip_params(::Type{<:NamedTuple{names}}) where {names} = NamedTuple{names}

test/runtests.jl

@@ -20,11 +20,25 @@ end
 
 @testset "indexstyle" begin
     @inferred IndexStyle(StructArray(a=rand(10,10), b=view(rand(100,100), 1:10, 1:10)))
-    T = typeof(StructArray(a=rand(10,10), b=view(rand(100,100), 1:10, 1:10)))
+    s = StructArray(a=rand(10,10), b=view(rand(100,100), 1:10, 1:10))
+    T = typeof(s)
     @test IndexStyle(T) === IndexCartesian()
+    @test StructArrays._best_index(T) == CartesianIndex{2}
+    @test s[100] == s[10, 10] == (a=s.a[10,10], b=s.b[10,10])
+    s[100] = (a=1, b=1)
+    @test s[100] == s[10, 10] == (a=1, b=1)
+    s[10, 10] = (a=0, b=0)
+    @test s[100] == s[10, 10] == (a=0, b=0)
     @inferred IndexStyle(StructArray(a=rand(10,10), b=rand(10,10)))
-    T = typeof(StructArray(a=rand(10,10), b=rand(10,10)))
+    s = StructArray(a=rand(10,10), b=rand(10,10))
+    T = typeof(s)
     @test IndexStyle(T) === IndexLinear()
+    @test StructArrays._best_index(T) == Int
+    @test s[100] == s[10, 10] == (a=s.a[10,10], b=s.b[10,10])
+    s[100] = (a=1, b=1)
+    @test s[100] == s[10, 10] == (a=1, b=1)
+    s[10, 10] = (a=0, b=0)
+    @test s[100] == s[10, 10] == (a=0, b=0)
 end
 
 @testset "replace_storage" begin