Fix OffsetArray support and clean CategoricalArray support

Invalid indices were used with `OffsetArray`s as 1-based indexing was assumed.
Fix this, and always wrap them in a `ToArrow` objet so that they are consistently
turned into 1-based arrays.

This allows dropping special code for `CategoricalArray` in favor of using the
standard `DataAPI.refpool` API combined with the Arrow extension point added to
CategoricalArrays.

Author: nalimilan

src/ArrowTypes/src/ArrowTypes.jl

@@ -404,17 +404,15 @@ concrete_or_concreteunion(T) =
 function ToArrow(x::A) where {A}
     S = eltype(A)
     T = ArrowType(S)
-    if S === T && concrete_or_concreteunion(S)
+    fi = firstindex(x)
+    if S === T && concrete_or_concreteunion(S) && fi == 1
         return x
     elseif !concrete_or_concreteunion(T)
         # arrow needs concrete types, so try to find a concrete common type, preferring unions
         if isempty(x)
             return Missing[]
         end
-        T = typeof(toarrow(x[1]))
-        for i = 2:length(x)
-            @inbounds T = promoteunion(T, typeof(toarrow(x[i])))
-        end
+        T = mapreduce(typeof ∘ toarrow, promoteunion, x)
         if T === Missing && concrete_or_concreteunion(S)
             T = promoteunion(T, typeof(toarrow(default(S))))
         end
@@ -442,6 +440,7 @@ function _convert(::Type{T}, x) where {T}
         return convert(T, x)
     end
 end
-Base.getindex(x::ToArrow{T}, i::Int) where {T} = _convert(T, toarrow(getindex(x.data, i)))
+Base.getindex(x::ToArrow{T}, i::Int) where {T} =
+    _convert(T, toarrow(getindex(x.data, i + firstindex(x.data) - 1)))
 
 end # module ArrowTypes

src/ArrowTypes/test/Project.toml

@@ -16,6 +16,7 @@
 # under the License.
 
 [deps]
+OffsetArrays = "6fe1bfb0-de20-5000-8ca7-80f57d26f881"
 Sockets = "6462fe0b-24de-5631-8697-dd941f90decc"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"

src/ArrowTypes/test/runtests.jl

@@ -15,6 +15,6 @@
 # specific language governing permissions and limitations
 # under the License.
 
-using Test, ArrowTypes, UUIDs, Sockets
+using Test, ArrowTypes, UUIDs, Sockets, OffsetArrays
 
 include("tests.jl")

src/ArrowTypes/test/tests.jl

@@ -192,6 +192,16 @@ end
         @test eltype(x) == Union{Float64,String}
         @test x == [1.0, 3.14, "hey"]
 
+        x = ArrowTypes.ToArrow(OffsetArray([1, 2, 3], -3:-1))
+        @test x isa ArrowTypes.ToArrow{Int,OffsetVector{Int,Vector{Int}}}
+        @test eltype(x) == Int
+        @test x == [1, 2, 3]
+
+        x = ArrowTypes.ToArrow(OffsetArray(Any[1, 3.14], -3:-2))
+        @test x isa ArrowTypes.ToArrow{Float64,OffsetVector{Any,Vector{Any}}}
+        @test eltype(x) == Float64
+        @test x == [1, 3.14]
+
         @testset "respect non-missing concrete type" begin
             struct DateTimeTZ
                 instant::Int64

src/arraytypes/dictencoding.jl

@@ -227,22 +227,8 @@ function arrowvector(
             refa = DataAPI.refarray(x)
             inds = copyto!(similar(Vector{signedtype(length(pool))}, length(refa)), refa)
         end
-        # horrible hack? yes. better than taking CategoricalArrays dependency? also yes.
-        if typeof(pool).name.name == :CategoricalRefPool
-            if eltype(x) >: Missing
-                pool = vcat(missing, DataAPI.levels(x))
-            else
-                pool = DataAPI.levels(x)
-                for i = 1:length(inds)
-                    @inbounds inds[i] -= 1
-                end
-            end
-        else
-            # adjust to "offset" instead of index
-            for i = 1:length(inds)
-                @inbounds inds[i] -= 1
-            end
-        end
+        # adjust to "offset" instead of index
+        inds .-= firstindex(refa)
         data = arrowvector(
             pool,
             i,
@@ -278,11 +264,7 @@ function arrowvector(
             )
             deltas = eltype(x)[]
             inds = Vector{ET}(undef, len)
-            categorical = typeof(x).name.name == :CategoricalArray
             for (j, val) in enumerate(x)
-                if categorical
-                    val = get(val)
-                end
                 @inbounds inds[j] = get!(pool, val) do
                     push!(deltas, val)
                     return length(pool)