ARROW-3674: [Go] Implement Date32 and Date64 array types#3170
Conversation
Implement both Date32 and Date64 types for arrays. Also resolves
ARROW-3675. Unit tests follow the same pattern as the existing float64
and Time{32,64} tests.
Codecov Report
@@ Coverage Diff @@
## master #3170 +/- ##
===========================================
- Coverage 87.09% 67.87% -19.23%
===========================================
Files 495 58 -437
Lines 69704 4012 -65692
===========================================
- Hits 60708 2723 -57985
+ Misses 8897 1186 -7711
- Partials 99 103 +4
Continue to review full report at Codecov.
|
|
Thanks @sbinet for taking a look, and linking the other JIRA ticket. Given that I don't have write access, is there anything else I need to do to get this merged? |
|
I'd like to wait for another pair of eyes :) |
|
Sounds good, thanks! |
| "Default": "0", | ||
| "Size": "8", | ||
| "Opt": { | ||
| "Parametric": true |
| "Default": "0", | ||
| "Size": "4", | ||
| "Opt": { | ||
| "Parametric": true |
There was a problem hiding this comment.
AFAIK, they're not parametric.
There was a problem hiding this comment.
Makes sense, there's only a single Date32 unit (same as Date64).
However, when I remove this, the generated code in numericbuilder.gen.go uses arrow.PrimitiveTypes.Date32 within the function Date32Builder.newData, but Date32 isn't added to PrimitiveTypes in datatype_numeric.gen.go. Is this a bug in the generator or is there something else I need to add? Are both "QualifiedType" and "InternalType" meant to stay?
|
Ok, I figured it out. Should I squash these commits into one, or keep as it? |
|
No need to squash |
|
@sbinet or @stuartcarnie any final words about this one? I'll merge in a day or two if I don't hear back |
|
Looks great to me, too 👌 |
Implement both Date32 and Date64 types for arrays. Also resolves ARROW-3675. Unit tests follow the same pattern as the existing float64 and Time{32,64} tests.