Arrow.jl 2.6 breaks Legolas.jl's tests

[ericphanson]

Arrow.jl 2.6 breaks Legolas.jl's tests

[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)

[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

[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?

[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 beacon-biosignals/Legolas.jl#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