Arrow.jl 2.6 breaks Legolas.jl's tests

[ericphanson]

As well as Onda.jl

E.g. this test fails with the error:

┌ Warning: The `Tables.Schema` of the `Arrow.Table` read via `Legolas.read(io_or_path)` does not appear to
│ comply with the `Legolas.SchemaVersion` indicated by the table's metadata (`SchemaVersion("test.parent", 1)`). Try invoking
│ `Legolas.read(io_or_path; validate=false)` to inspect the table.
└ @ Legolas ~/Legolas.jl/src/tables.jl:171
miscellaneous Legolas/src/tables.jl tests: Error During Test at /Users/eph/Legolas.jl/test/runtests.jl:468
  Test threw exception
  Expression: t == [NamedTuple(ParentV1(r)) for r = Tables.rows(Legolas.read(path))]
  ArgumentError: Tables.Schema violates Legolas schema `test.parent@1`:
   - Incorrect type: `x` expected `<:Vector`, found `SubArray{Int64, 1, Arrow.Primitive{Int64, Vector{Int64}}, Tuple{UnitRange{Int64}}, true}`
  Provided Tables.Schema:
   :x  SubArray{Int64, 1, Arrow.Primitive{Int64, Vector{Int64}}, Tuple{UnitRange{Int64}}, true}
   :y  String
  Stacktrace:
   [1] validate(ts::Tables.Schema{(:x, :y), Tuple{SubArray{Int64, 1, Arrow.Primitive{Int64, Vector{Int64}}, Tuple{UnitRange{Int64}}, true}, String}}, sv::SchemaVersion{Symbol("test.parent"), 1})
     @ Legolas ~/Legolas.jl/src/schemas.jl:312
   [2] read(io_or_path::MyPath; validate::Bool)
     @ Legolas ~/Legolas.jl/src/tables.jl:169
   [3] read(io_or_path::MyPath)
     @ Legolas ~/Legolas.jl/src/tables.jl:159
   [4] macro expansion
     @ ~/.julia/juliaup/julia-1.8.5+0.aarch64.apple.darwin14/share/julia/stdlib/v1.8/Test/src/Test.jl:464 [inlined]
   [5] macro expansion
     @ ~/Legolas.jl/test/runtests.jl:468 [inlined]
   [6] macro expansion
     @ ~/.julia/juliaup/julia-1.8.5+0.aarch64.apple.darwin14/share/julia/stdlib/v1.8/Test/src/Test.jl:1363 [inlined]
   [7] top-level scope
     @ ~/Legolas.jl/test/runtests.jl:459
┌ Warning: The `Tables.Schema` of the `Arrow.Table` read via `Legolas.read(io_or_path)` does not appear to
│ comply with the `Legolas.SchemaVersion` indicated by the table's metadata (`SchemaVersion("haha", 3)`). Try invoking
│ `Legolas.read(io_or_path; validate=false)` to inspect the table.
└ @ Legolas ~/Legolas.jl/src/tables.jl:171

(Note, this test fails with both ArrowTypes v2.2.0 and ArrowTypes v2.1.0, so I think it is Arrow and not ArrowTypes that is the isuse)

[ericphanson]

Maybe we should yank v2.6 from General? Since it wasn't meant to be a breaking release. cc @quinnj

[Moelf]

What's this actually testing, the :< Vector seems way too strong

[ericphanson]

I think it calls convert first, but then, yeah, it expects a Vector{Int} for the element (not for the whole column, just for the individual element). That is what we got on Arrow 2.5:

julia> Arrow.Table(read(path))
Arrow.Table with 2 rows, 2 columns, and schema:
 :x  Vector{Int64} (alias for Array{Int64, 1})
 :y  String

with metadata given by a Base.ImmutableDict{String, String} with 1 entry:
  "legolas_schema_qualified" => "test.parent@1"

julia> Arrow.Table(read(path)).x
2-element Arrow.List{Vector{Int64}, Int32, Arrow.Primitive{Int64, Vector{Int64}}}:
 [1, 2]
 [3, 4]

julia> Arrow.Table(read(path)).x |> typeof
Arrow.List{Vector{Int64}, Int32, Arrow.Primitive{Int64, Vector{Int64}}}

julia> Arrow.Table(read(path)).x[1] |> typeof
Vector{Int64} (alias for Array{Int64, 1})

edit: it seems to not call convert first in my testing 🤔

[Moelf]

Yeah well that's too strong, when you have a jagged column, it's best to not return Vector for each element because that copies.

Arrow stores jagged column as one data column and one offset column, like VectorOfVectors from (ArrayOrArrays.jl). So really the promise is that each element will be :<AbstractVector

[ericphanson]

What is a jagged column?

[Moelf]

A column that's vector of vectors (of various length )

[ericphanson]

Hm ok. I think we should be trying to convert at least before validating, I agree it is very strict otherwise. Legolas has a mechanism for related things, I think maybe we need to add some definitions like this:

Legolas.accepted_field_type(::Legolas.SchemaVersion, ::Type{Vector{String}}) = AbstractVector{<:AbstractString}
Legolas.accepted_field_type(::Legolas.SchemaVersion, ::Type{Vector{T}}) where T = AbstractVector{<:T}
Legolas.accepted_field_type(::Legolas.SchemaVersion, ::Type{Vector}) = AbstractVector

[Moelf]

Yeah, in any case Arrow schema is just a logical construction, most likely can't be mapped to Julia types one to one and shouldn't (other than the primitive bit types)

So we need some kind of Julia side logical schema in the end. (For vector it's nicely covered by AbstractVector, because Julia's type hierarchy is kinda designed for these technical usage)

[quinnj]

FYI, we tweaked the definition here to accommodate the chance that list-type columns will return SubArray now

[hannahilea]

Thanks for the clarification, all. I opened https://github.com/beacon-biosignals/Legolas.jl/pull/89 with the fix proposed by @ericphanson, since it (now!) seems clearer that the failing Legolas tests are due to Legolas being too(?) strict to begin with, rather than any change introduced by Arrow 2.6.

[ericphanson]

Ok let's close this one then, thanks for the help everyone