GH-50437: [Ruby] Add ArrowFormat::*IntervalArray.new(values)#50459
Conversation
|
|
github-actions
Bot
added
Component: Ruby
awaiting committer review
23de210 to
4983f2d
Compare
There was a problem hiding this comment.
Pull request overview
Adds Ruby ArrowFormat::*IntervalArray.new(values) convenience constructors for Arrow interval arrays, enabling simpler creation of year-month, day-time, and month-day-nano interval arrays from Ruby value collections.
Changes:
- Added interval type
pack_templatesupport and updated intervalType#build_arrayto construct arrays without passing an explicit type instance. - Added
.typesingleton hooks on interval array classes to support.new(values)construction. - Added custom
#each(and packing helpers) for day-time and month-day-nano interval arrays, plus new test coverage for interval-array initialization and equality.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ruby/red-arrow-format/lib/arrow-format/type.rb | Adds pack_template for interval types and updates build_array to align with .new(values) constructors. |
| ruby/red-arrow-format/lib/arrow-format/array.rb | Refactors packing to use pack_value and adds interval-specific #each/packing behavior. |
| ruby/red-arrow-format/test/test-year-month-interval-array.rb | Adds tests for year-month interval array initialization and equality. |
| ruby/red-arrow-format/test/test-day-time-interval-array.rb | Adds tests for day-time interval array initialization and equality. |
| ruby/red-arrow-format/test/test-month-day-nano-interval-array.rb | Adds tests for month-day-nano interval array initialization and equality. |
| def pack_value(value, template) | ||
| [value || 0].pack(template) | ||
| end |
| assert_not_equal(array1, array2.slice(1, 3)) | ||
| end | ||
| end | ||
| end |
| assert_not_equal(array1, array2.slice(1, 3)) | ||
| end | ||
| end | ||
| end |
4983f2d to
de8ba54
Compare
| class YearMonthIntervalArray < IntervalArray | ||
| class << self | ||
| def type | ||
| YearMonthIntervalType.singleton | ||
| end | ||
| end |
| def each(&block) | ||
| return to_enum(__method__) {@size} unless block_given? | ||
|
|
||
| each_value = Enumerator.new(@size) do |yielder| | ||
| offset = element_size * @offset | ||
| @values_buffer. |
Reranko05
left a comment
Reranko05
left a comment
There was a problem hiding this comment.
Would it make sense to add dedicated #each tests for DayTimeIntervalArray and MonthDayNanoIntervalArray, or is to_a coverage sufficient?
|
It should be covered by sliced |
Good catch! I've fixed it. |
|
I'll merge this. |
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit eea7369. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
Building a year month/day time/month day nano interval Arrow array from Ruby objects is convenient.
What changes are included in this PR?
ArrowFormat::YearMonthIntervalArray.new(values)ArrowFormat::DayTimetntervalArray.new(values)ArrowFormat::DayTimeIntervalArray#eachArrowFormat::MonthDayNanoIntervalArray.new(values)ArrowFormat::MonthDayNanoIntervalArray#eachAre these changes tested?
Yes.
Are there any user-facing changes?
Yes.
ArrowFormat::*IntervalArray.new(values)#50437