Skip to content
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

missing methods for fromarrow(::Type{Time}, x::Arrow.Time{<:Arrow.Flatbuf.TimeUnit.T, <:Integer}) #526

Closed
palday opened this issue Sep 24, 2024 · 2 comments · Fixed by #527

Comments

@palday
Copy link
Contributor

palday commented Sep 24, 2024

I did not create table in question, but the column in question contains a time stored as Arrow.Time{Arrow.Flatbuf.TimeUnit.MICROSECOND, Int64} which errors upon access.

This can be fixed by adding the methods

function fromarrow(::Type{Time}, x::Arrow.Time{Arrow.Flatbuf.TimeUnit.MICROSECOND, <:Integer})
   return Time(0, 0, 0) + Microsecond(x.x)
end

function fromarrow(::Type{Time}, x::Arrow.Time{Arrow.Flatbuf.TimeUnit.MILLISECOND, <:Integer})
    return Time(0, 0, 0) + Millisecond(x.x)
 end

 function fromarrow(::Type{Time}, x::Arrow.Time{Arrow.Flatbuf.TimeUnit.NANOSECOND, <:Integer})
    return Time(0, 0, 0) + Nanosecond(x.x)
 end

 function fromarrow(::Type{Time}, x::Arrow.Time{Arrow.Flatbuf.TimeUnit.SECOND, <:Integer})
    return Time(0, 0, 0) + Second(x.x)
 end

I can open a PR adding these methods if there is no other preferred solution

@palday
Copy link
Contributor Author

palday commented Sep 24, 2024

Possibly related to #363

@ararslan
Copy link

Looks like the right convert method exists and is general enough to handle all of the above cases, it's just fromarrow that's currently too specific: it's assuming all incoming Arrow.Times are nanoseconds. I think this is all that's needed:

diff --git a/src/eltypes.jl b/src/eltypes.jl
index ffc53c0..6071365 100644
--- a/src/eltypes.jl
+++ b/src/eltypes.jl
@@ -251,7 +251,7 @@ ArrowTypes.toarrow(x::Dates.Time) = convert(TIME, x)
 const TIME_SYMBOL = Symbol("JuliaLang.Time")
 ArrowTypes.arrowname(::Type{Dates.Time}) = TIME_SYMBOL
 ArrowTypes.JuliaType(::Val{TIME_SYMBOL}, S) = Dates.Time
-ArrowTypes.fromarrow(::Type{Dates.Time}, x::TIME) = convert(Dates.Time, x)
+ArrowTypes.fromarrow(::Type{Dates.Time}, x::Time) = convert(Dates.Time, x)
 ArrowTypes.default(::Type{Dates.Time}) = Dates.Time(1, 1, 1)

 struct Timestamp{U,TZ} <: ArrowTimeType

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants