apache / arrow-go

Official Go implementation of Apache Arrow
https://arrow.apache.org/
Apache License 2.0
43 stars 7 forks source link

[Go] Date32 and Date64 String() methods return numbers instead of dates #63

Open hermanschaaf opened 1 year ago

hermanschaaf commented 1 year ago

Describe the bug, including details regarding any error messages, version, and platform.

The String methods on the Go array.Date32 and array.Date64 types return strings formatted as the underlying integers, rather than strings formatted as dates as in other implementations.

For example, this code in Go:

package main

import (
    "fmt"

    "github.com/apache/arrow/go/v12/arrow"
    "github.com/apache/arrow/go/v12/arrow/array"
    "github.com/apache/arrow/go/v12/arrow/memory"
)

func main() {
    db := array.NewDate32Builder(memory.DefaultAllocator)
    defer db.Release()
    db.AppendValues([]arrow.Date32{1, 2, 3}, nil)
    arr := db.NewArray()
    defer arr.Release()
    fmt.Println(arr)
}

produces the following output:

[1 2 3]

More or less equivalent code in Python:

import pyarrow as pa

date_array = pa.array([1, 2, 3], type=pa.date32())
print(date_array)

produces:

[
  1970-01-02,
  1970-01-03,
  1970-01-04
]

I think the Go implementation's dates should also return formatted dates?

Component(s)

Go

hermanschaaf commented 1 year ago

I can submit a patch for this: just want to check that you agree this is a bug first.

zeroshade commented 1 year ago

So, the arrow.Date32 and arrow.Date64 types have FormattedString methods which return the actual date instead of the numeric value. I'm not opposed to just renaming them as String() so that it becomes the default string representation for those types and it should be pretty easy to do.

Alternately, the String() method of those arrays could be modified to call FormattedString instead of just writing the numbers to the buffer.

Honestly, I'd probably label this as an enhancement rather than a bug, because the current behavior is the intended behavior.

hermanschaaf commented 1 year ago

@zeroshade Great, thanks for the context! Yeah, I think the Go implementation here should probably match the C++/Python ones, and print the formatted string, so that when you print the array you get the actual dates. But if it was intended to work the way it is now then maybe it's fine. It's good to know there is at least a FormattedString function that could be used.

zeroshade commented 1 year ago

@hermanschaaf I think that bringing them into alignment is a perfectly reasonable thing to do, and if you want to file a PR to implement one of the suggestions I made, I'd happily review and support such a thing. (or if you have a better idea than my off the cuff suggestions). Let's mark this as an enhancement request for now and just ping me if you decide to file the PR.