-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Register the Interval structs with Arrow.jl via Requires.jl #156
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #156 +/- ##
==========================================
- Coverage 76.55% 76.25% -0.30%
==========================================
Files 11 11
Lines 516 518 +2
==========================================
Hits 395 395
- Misses 121 123 +2
Continue to review full report at Codecov.
|
I guess this would require us to drop testing on Julia 1.0 since Arrow requires 1.3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be good with the last correction
Co-authored-by: Curtis Vogt <curtis.vogt@gmail.com>
I'm going to run a couple experiments with this today and either merge as-is or make some changes. |
Looks like the parametric information isn't being stored which is problematic: julia> using Arrow
julia> using Dates, Intervals, TimeZones
julia> table = (interval = [
Interval{Closed,Closed}(1,2),
Interval{Open,Closed}(1,2),
Interval{Closed,Unbounded}(1,nothing),
],)
(interval = Interval{Int64,L,R} where R<:Bound where L<:Bound[Interval{Int64,Closed,Closed}(1, 2), Interval{Int64,Open,Closed}(1, 2), Interval{Int64,Closed,Unbounded}(1, nothing)],)
julia> Arrow.write("data.arrow", table)
julia> loaded = Arrow.Table("data.arrow");
julia> loaded.interval
3-element Arrow.Struct{Interval{Int64,L,R} where R<:Bound where L<:Bound,Tuple{Arrow.Primitive{Int64,Array{Int64,1}},Arrow.Primitive{Int64,Array{Int64,1}}}}:
Interval{Int64,Closed,Closed}(1, 2)
Interval{Int64,Closed,Closed}(1, 2)
Interval{Int64,Closed,Closed}(1, 1) |
I've made issues with Arrow for the two problems I've discovered: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to address Arrow issues
One thing to consider with this addition is that Arrow.jl doesn't handle type changes very well.
Options:
Intervalv1
,Intervalv2
) and appropriate upgrade conversions.apache/arrow-julia#133