Copilot commented on code in PR #50459:
URL: https://github.com/apache/arrow/pull/50459#discussion_r3555907307
##########
ruby/red-arrow-format/lib/arrow-format/array.rb:
##########
@@ -491,29 +496,80 @@ class IntervalArray < TemporalArray
end
class YearMonthIntervalArray < IntervalArray
+ class << self
+ def type
+ YearMonthIntervalType.singleton
+ end
+ end
Review Comment:
Adding `.type` to the interval array classes makes them follow the
`PrimitiveArray` “fixed type” initializer path, which means the
previously-possible signature `*IntervalArray.new(type, values)` will now raise
`ArgumentError` (because the explicit type becomes an extra argument). If this
was used externally as a workaround before this PR, it becomes a breaking
change. Consider accepting and ignoring a leading `Type` argument for interval
arrays to preserve backwards compatibility while still supporting
`.new(values)`.
##########
ruby/red-arrow-format/lib/arrow-format/array.rb:
##########
@@ -491,29 +496,80 @@ class IntervalArray < TemporalArray
end
class YearMonthIntervalArray < IntervalArray
+ class << self
+ def type
+ YearMonthIntervalType.singleton
+ end
+ end
end
class DayTimeIntervalArray < IntervalArray
+ class << self
+ def type
+ DayTimeIntervalType.singleton
+ end
+ end
+
def to_a
return [] if empty?
offset = element_size * @offset
values = @values_buffer.
each(@type.buffer_type, offset, @size * 2).
each_slice(2).
- collect do |(_, day), (_, time)|
- [day, time]
+ collect do |(_, day), (_, millisecond)|
+ [day, millisecond]
end
apply_validity(values)
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.
Review Comment:
The newly added `#each` implementations for interval arrays are not covered
by the new tests (tests only exercise `#to_a` and `#==`). Please add tests that
validate `#each` yields the expected tuples and respects nulls (including
enumerator behavior when called without a block) for both DayTimeIntervalArray
and MonthDayNanoIntervalArray.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
