Merge pull request #121 from JuliaArrays/pv/static

foreachfield and staticschema cleanup

Author: Pietro Vertechi

src/StructArrays.jl

@@ -1,7 +1,6 @@
 module StructArrays
 
 using Base: tuple_type_cons, tuple_type_head, tuple_type_tail, tail
-using Tables: getcolumn, Tables
 
 export StructArray, StructVector, LazyRow, LazyRows
 export collect_structarray, fieldarrays

src/lazy.jl

@@ -40,8 +40,6 @@ Base.getproperty(s::LazyRows, key::Symbol) = getproperty(parent(s), key)
 Base.getproperty(s::LazyRows, key::Int) = getproperty(parent(s), key)
 Base.propertynames(c::LazyRows) = propertynames(parent(c))
 
-staticschema(::Type{LazyRows{T, N, C, I}}) where {T, N, C, I} = staticschema(StructArray{T, N, C, I})
-
 Base.size(v::LazyRows) = size(parent(v))
 Base.getindex(v::LazyRows{<:Any, <:Any, <:Any, Int}, i::Int) = LazyRow(parent(v), i)
 Base.getindex(v::LazyRows{<:Any, <:Any, <:Any, CartesianIndex{N}}, i::Vararg{Int, N}) where {N} = LazyRow(parent(v), CartesianIndex(i))

src/structarray.jl

@@ -126,7 +126,6 @@ fieldarrays(s::StructArray) = getfield(s, :fieldarrays)
 Base.getproperty(s::StructArray, key::Symbol) = getfield(fieldarrays(s), key)
 Base.getproperty(s::StructArray, key::Int) = getfield(fieldarrays(s), key)
 Base.propertynames(s::StructArray) = propertynames(fieldarrays(s))
-staticschema(::Type{<:StructArray{T}}) where {T} = staticschema(T)
 
 Base.size(s::StructArray) = size(fieldarrays(s)[1])
 Base.size(s::StructArray{<:Any, <:Any, <:EmptyTup}) = (0,)

src/tables.jl

@@ -1,17 +1,32 @@
+import Tables
+
 Tables.isrowtable(::Type{<:StructArray}) = true
 
 Tables.columnaccess(::Type{<:StructArray}) = true
 Tables.columns(s::StructArray) = fieldarrays(s)
-Tables.getcolumn(s::StructArray, i::Int) = getproperty(s, i)
-Tables.schema(s::StructArray) = Tables.Schema(staticschema(eltype(s)))
+Tables.schema(s::StructArray) = _schema(staticschema(eltype(s)))
+
+_schema(::Type{NT}) where {NT<:NamedTuple} = Tables.Schema(NT)
+# make schema for unnamed case
+function _schema(::Type{T}) where {T<:NTuple{N, Any}} where N
+    return Tables.Schema{ntuple(identity, N), T}
+end
+
+function try_compatible_columns(rows::R, s::StructArray) where {R}
+    Tables.isrowtable(rows) && Tables.columnaccess(rows) || return nothing
+    T = eltype(rows)
+    hasfields(T) || return nothing
+    NT = staticschema(T)
+    _schema(NT) == Tables.schema(rows) || return nothing
+    table = Tables.columns(rows)
+    fieldnames(NT) == propertynames(table) ? table : nothing
+end
 
 function Base.append!(s::StructVector, rows)
-    if Tables.isrowtable(rows) && Tables.columnaccess(rows)
+    table = try_compatible_columns(rows, s)
+    if table !== nothing
         # Input `rows` is a container of rows _and_ satisfies column
         # table interface.  Thus, we can add the input column-by-column.
-        table = Tables.columns(rows)
-        isempty(_setdiff(propertynames(s), Tables.columnnames(rows))) ||
-            _invalid_columns_error(s, rows)
         foreachfield(append!, s, table)
         return s
     else
@@ -20,12 +35,3 @@ function Base.append!(s::StructVector, rows)
         return foldl(push!, rows; init = s)
     end
 end
-
-@noinline function _invalid_columns_error(s, rows)
-    missingnames = setdiff!(collect(Tables.columnnames(rows)), propertynames(s))
-    throw(ArgumentError(string(
-        "Cannot append rows from `$(typeof(rows))` to `$(typeof(s))` due to ",
-        "missing column(s):\n",
-        join(missingnames, ", "),
-    )))
-end

src/utils.jl

@@ -21,6 +21,13 @@ function buildfromschema(initializer, ::Type{T}, ::Type{NT}) where {T, NT<:Tup}
     StructArray{T}(nt)
 end
 
+@static if VERSION < v"1.2.0"
+    @inline _getproperty(v::Tuple, field) = getfield(v, field)
+    @inline _getproperty(v, field) = getproperty(v, field)
+else
+    const _getproperty = getproperty
+end
+
 function _foreachfield(names, L)
     vars = ntuple(i -> gensym(), L)
     exprs = Expr[]
@@ -29,19 +36,19 @@ function _foreachfield(names, L)
     end
     for field in names
         sym = QuoteNode(field)
-        args = [Expr(:call, :getcolumn, var, sym) for var in vars]
+        args = [Expr(:call, :_getproperty, var, sym) for var in vars]
         push!(exprs, Expr(:call, :f, args...))
     end
     push!(exprs, :(return nothing))
     return Expr(:block, exprs...)
 end
 
-@generated foreachfield(::Type{<:NamedTuple{names}}, f, xs::Vararg{Any, L}) where {names, L} =
+@generated foreachfield_gen(::NamedTuple{names}, f, xs::Vararg{Any, L}) where {names, L} =
     _foreachfield(names, L)
-@generated foreachfield(::Type{<:NTuple{N, Any}}, f, xs::Vararg{Any, L}) where {N, L} =
+@generated foreachfield_gen(::NTuple{N, Any}, f, xs::Vararg{Any, L}) where {N, L} =
     _foreachfield(Base.OneTo(N), L)
 
-foreachfield(f, x::T, xs...) where {T} = foreachfield(staticschema(T), f, x, xs...)
+foreachfield(f, x::StructArray, xs...) = foreachfield_gen(fieldarrays(x), f, x, xs...)
 
 """
 `iscompatible(::Type{S}, ::Type{V}) where {S, V<:AbstractArray}`
@@ -126,11 +133,3 @@ hasfields(::Type{<:NTuple{N, Any}}) where {N} = true
 hasfields(::Type{<:NamedTuple{names}}) where {names} = true
 hasfields(::Type{T}) where {T} = !isabstracttype(T)
 hasfields(::Union) = false
-
-_setdiff(a, b) = setdiff(a, b)
-
-@inline _setdiff(::Tuple{}, ::Tuple{}) = ()
-@inline _setdiff(::Tuple{}, ::Tuple) = ()
-@inline _setdiff(a::Tuple, ::Tuple{}) = a
-@inline _setdiff(a::Tuple, b::Tuple) = _setdiff(_exclude(a, b[1]), Base.tail(b))
-@inline _exclude(a, b) = foldl((ys, x) -> x == b ? ys : (ys..., x), a; init = ())

test/runtests.jl

@@ -24,7 +24,7 @@ end
 
 @testset "utils" begin
     t = StructArray(rand(ComplexF64, 2, 2))
-    T = staticschema(typeof(t))
+    T = staticschema(eltype(t))
     @test StructArrays.eltypes(T) == NamedTuple{(:re, :im), Tuple{Float64, Float64}}
     @test StructArrays.map_params(eltype, T) == NamedTuple{(:re, :im), Tuple{Float64, Float64}}
     @test StructArrays.map_params(eltype, StructArrays.astuple(T)) == Tuple{Float64, Float64}
@@ -336,10 +336,10 @@ end
     @test Tables.rowaccess(typeof(s))
     @test Tables.columnaccess(s)
     @test Tables.columnaccess(typeof(s))
-    @test Tables.getcolumn(s, 1) == [1]
-    @test Tables.getcolumn(s, :a) == [1]
-    @test Tables.getcolumn(s, 2) == ["test"]
-    @test Tables.getcolumn(s, :b) == ["test"]
+    @test Tables.getcolumn(Tables.columns(s), 1) == [1]
+    @test Tables.getcolumn(Tables.columns(s), :a) == [1]
+    @test Tables.getcolumn(Tables.columns(s), 2) == ["test"]
+    @test Tables.getcolumn(Tables.columns(s), :b) == ["test"]
     @test append!(StructArray([1im]), [(re = 111, im = 222)]) ==
         StructArray([1im, 111 + 222im])
     @test append!(StructArray([1im]), (x for x in [(re = 111, im = 222)])) ==
@@ -579,7 +579,7 @@ end
     rows = LazyRows(s)
     @test propertynames(rows) == (:re, :im)
     @test propertynames(rows[1]) == (:re, :im)
-    @test staticschema(typeof(rows)) == staticschema(eltype(rows)) == staticschema(ComplexF64)
+    @test staticschema(eltype(rows)) == staticschema(ComplexF64)
     @test getproperty(rows, 1) isa Matrix{Float64}
     @test getproperty(rows, :re) isa Matrix{Float64}
     @test IndexStyle(rows) isa IndexCartesian
@@ -593,7 +593,7 @@ end
     rows = LazyRows(s)
     @test propertynames(rows) == (:re, :im)
     @test propertynames(rows[1]) == (:re, :im)
-    @test staticschema(typeof(rows)) == staticschema(eltype(rows)) == staticschema(ComplexF64)
+    @test staticschema(eltype(rows)) == staticschema(ComplexF64)
     @test getproperty(rows, 1) isa Matrix{Float64}
     @test getproperty(rows, :re) isa Matrix{Float64}
     @test IndexStyle(rows) isa IndexLinear