improve tables support

[aplavin]

StructArrays constructor fundamentally cannot convert an arbitrary Tables.jl table to a StructArray. This would interfere with current, and totally sensible, collection handling: Tables.jl don't provide a common abstract type to dispatch.

However, I think it makes sense to support tables when dispatching is straightforward. Here, I add direct construction support for AbstractColumns. Its subtypes include numerous columnar tables defined in Tables.jl and in other packages. As a concrete motivating example, this enables CSV.read("file.csv", StructArray).

UPD: also, this PR adds a separate function StructArrays.fromtable to support constructing from arbitrary Tables objects, and Tables.materializer support.

[aplavin]

Test failures seem irrelevant.

[aplavin]

Bump

[aplavin]

bump!

[piever]

Sorry for the delay in the review merge of the various PRs! So, here I agree that it makes sense to have a simpler way to do this, without asking users to do StructArray(Tables.columntable(x)), but somehow dispatching on AbstractColumns seems not ideal (or at least the docs of AbstractColumns suggest against it). OTOH, using the Tables machinery on StructArray(x::Any) seems like it could be confusing for a variety of reasons (for instance, a StructArray has more structure than a table as it preserves the eltype).

Possible options:

1. See if it is possible to have a generic wrapper type Table to simply declare that an object is a table and change CSV.read(input, sink) = sink(Table(CSV.File(input))) in CSV.jl. Then, StructArray(t::Table) can do the right thing. Scratched, as the materializer approach mentioned below seems easier.
2. Simply create a StructArrays.fromtable(x) = StructArray(columntable(x)) method. CSV.read(input, StructArrays.fromtable) seems like a reasonable interface to me.

cc: @quinnj

[aplavin]

As I understand, dispatching on AbstractColumns is not harmful in any case. It's just that not all columnar tables are <: AbstractColumns, so this dispatch won't trigger for all tables. However, if something does subtype AbstractColumns, it surely is intended to be a table by the author of the type.

[aplavin]

using the Tables machinery on StructArray(x::Any) seems like it could be confusing

100%, please never do that :) Tables and arrays can easily conflict, and this is manifested as bugs in functions on arrays that shouldn't be affected on Tables at all - see #278 (because append! uses Tables machinery in StructArrays).

[piever]

I'm just a little concerned that it could create odd expectations that StructArray(x) works from tables only some of the time. I see how in practice that is already the case, but adding dispatches one by one may not be a fully satisfactory solution.

I'm adding a link to a similar discussion in CSV.jl JuliaData/CSV.jl#941 (comment). I would agree with the proposal linked above to instead have a simple fromtable function and maybe also a materializer(::Type{<:StructArray}) = fromtable. Then users could do

CSV.read(input, StructArrays.fromtable) # or even
CSV.read(input, materializer(StructArray))

If that is unintuitive, there is also the option of using materializer(sink) rather than sink in the default definition of CSV.read, but it seems like there was no consensus for that at the time (see above link).

[aplavin]

There are more and more cases when I wish this conversion was defined... AbstractColumns is not just for CSV, it's for many types defined both in Tables.jl and in other table-reading packages. Most recently, I encountered Arrow.jl: defining this method here would allow Arrow.Table(file) |> StructArray (now requires columntable inbetween).

StructArray is basically the type-stable columnar array + table in Julia, would be great to make interop as straightforward as possible. Yes, we cannot support all tables because there's nothing to dispatch on (and using Tables stuff without dispatch is quite bad, #278 for a specific bug it causes). But supporting more tables, those where it can be done cleanly, is a strict improvement IMO.

[aplavin]

Bump...
fromtable() and/or materializer() are also nice to have, but kinda orthogonal to this. Here, we can support conversion from many table types directly, without calling extra functions explicitly.

[aplavin]

gentle bump...

[aplavin]

bump

[aplavin]

Added fromtable and materializer following #291. Are there any remaining concerns regarding Tables support, or this can be merged?

[oschulz]

Thanks for this @aplavin !

[aplavin]

I'll take the liberty to merge this, following an advice on slack #arrays. The PR didn't get a strong "no" from maintainers, changes are nonbreaking, and quite local (ie not an overhaul).
Please let me know if anything is wrong, it's the first time I do this in JuliArrays.