search

apache / arrow-julia

Official Julia implementation of Apache Arrow
https://arrow.apache.org/julia/
Other
284 stars 59 forks source link

Don't treat Vector{UInt8} as Arrow Binary type #439

Closed quinnj closed 1 year ago

quinnj commented 1 year ago

Fixes #411. Alternative to #419.

This PR should be compatible with or without the ArrowTypes changes. I think it's fine to do compat things in Arrow like this as long as they don't get out of hand and we can eventually remove them as we bump required ArrowTypes versions and such.

The PR consists of not treating Vector{UInt8} as the Arrow Binary type, which is meant for "binary string"s. Julia has a pretty good match for that in Base.CodeUnits, so instead, we use that to write Binary and Vector{UInt8} is treated as a regular List of Primitive UInt8 type.

quinnj commented 1 year ago

Shoot, I forgot we run tests both ways; I'll add some more compat here

codecov-commenter commented 1 year ago

Codecov Report

Attention: Patch coverage is 96.87500% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.06%. Comparing base (e893c32) to head (a08bb51). Report is 47 commits behind head on main.

Files with missing lines Patch % Lines
src/arraytypes/list.jl 96.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #439 +/- ## ========================================== + Coverage 87.02% 87.06% +0.03% ========================================== Files 26 26 Lines 3261 3277 +16 ========================================== + Hits 2838 2853 +15 - Misses 423 424 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

quinnj commented 1 year ago

This should be ready to review/merge; cc: @Moelf @baumgold