kou commented on code in PR #51214:
URL: https://github.com/apache/arrow/pull/51214#discussion_r3955183277
##########
ruby/red-arrow-format/lib/arrow-format/record-batch.rb:
##########
@@ -143,6 +148,57 @@ def all_buffers_enumerator
end
private
+ def build_with_schema(schema, data)
+ fields = schema.fields
+ name_to_index = {}
+ fields.each_with_index do |field, i|
+ name_to_index[field.name] = i
+ end
+ if data.is_a?(Hash)
+ raw_columns = []
+ data.each do |name, values|
+ field_index = name_to_index[name.to_s]
+ raw_columns[field_index] = values if field_index
+ end
+ columns = fields.zip(raw_columns).collect do |field, values|
+ field.type.build_array(values || [])
+ end
+ else
+ raw_columns = fields.collect { [] }
+ data.each_with_index do |record, nth_record|
+ case record
+ when nil
+ when Hash
+ record.each do |name, value|
+ field_index = name_to_index[name.to_s]
+ raw_columns[field_index] << value if field_index
+ end
+ else
+ record.each_with_index do |value, field_index|
+ if field_index >= raw_columns.size
+ message = "row #{nth_record} has more values than schema
fields"
+ raise ArgumentError, message
+ end
Review Comment:
Can we do this out of `each_with_index` loop with `record.size`?
##########
ruby/red-arrow-format/lib/arrow-format/record-batch.rb:
##########
@@ -143,6 +148,57 @@ def all_buffers_enumerator
end
private
+ def build_with_schema(schema, data)
+ fields = schema.fields
+ name_to_index = {}
+ fields.each_with_index do |field, i|
+ name_to_index[field.name] = i
+ end
+ if data.is_a?(Hash)
+ raw_columns = []
+ data.each do |name, values|
+ field_index = name_to_index[name.to_s]
+ raw_columns[field_index] = values if field_index
+ end
+ columns = fields.zip(raw_columns).collect do |field, values|
+ field.type.build_array(values || [])
+ end
+ else
+ raw_columns = fields.collect { [] }
+ data.each_with_index do |record, nth_record|
+ case record
+ when nil
+ when Hash
+ record.each do |name, value|
+ field_index = name_to_index[name.to_s]
+ raw_columns[field_index] << value if field_index
+ end
+ else
+ record.each_with_index do |value, field_index|
+ if field_index >= raw_columns.size
+ message = "row #{nth_record} has more values than schema
fields"
+ raise ArgumentError, message
+ end
+ raw_columns[field_index] << value
+ end
+ end
+ raw_columns.each do |column|
+ column << nil if column.size != nth_record + 1
+ end
+ end
+ columns = fields.zip(raw_columns).collect do |field, values|
+ field.type.build_array(values)
+ end
+ end
+ n_rows = columns.first&.size || 0
+ all_n_rows = columns.collect(&:size)
+ if data.is_a?(Hash) && all_n_rows.uniq.size != 1
+ message = "inconsistent the number of rows: #{all_n_rows.join(", ")}"
+ raise ArgumentError, message
+ end
Review Comment:
Can we move this into the `if data.is_a?(Hash)` clause?
##########
ruby/red-arrow-format/test/test-record-batch.rb:
##########
@@ -26,6 +26,92 @@ def setup
end
sub_test_case("#initialize") do
+ sub_test_case("[Schema, values]") do
+ def setup
+ @schema = ArrowFormat::Schema.new([
+ ArrowFormat::Field.new(
+ "visible",
+
ArrowFormat::BooleanType.singleton,
+ ),
+ ArrowFormat::Field.new(
+ "count",
+
ArrowFormat::UInt32Type.singleton,
+ ),
+ ])
+ end
+
+ test("records") do
+ record_batch = ArrowFormat::RecordBatch.new(
+ @schema,
+ [
+ {visible: true, count: 1},
+ nil,
+ [false, 3],
+ ],
+ )
+ assert_equal(@schema, record_batch.schema)
+ assert_equal(ArrowFormat::BooleanArray,
+ record_batch.find_column("visible").class)
+ assert_equal(ArrowFormat::UInt32Array,
+ record_batch.find_column("count").class)
+ assert_equal([
+ {"visible" => true, "count" => 1},
+ {"visible" => nil, "count" => nil},
+ {"visible" => false, "count" => 3},
+ ],
+ record_batch.records.collect(&:to_h))
+ end
+
+ test("columns") do
+ record_batch = ArrowFormat::RecordBatch.new(
+ @schema,
+ {
+ visible: [true, nil, false],
+ "count" => [1, 2, nil],
+ },
+ )
+ assert_equal([
+ {"visible" => true, "count" => 1},
+ {"visible" => nil, "count" => 2},
+ {"visible" => false, "count" => nil},
Review Comment:
Ditto.
##########
ruby/red-arrow-format/test/test-record-batch.rb:
##########
@@ -26,6 +26,92 @@ def setup
end
sub_test_case("#initialize") do
+ sub_test_case("[Schema, values]") do
+ def setup
+ @schema = ArrowFormat::Schema.new([
+ ArrowFormat::Field.new(
+ "visible",
+
ArrowFormat::BooleanType.singleton,
+ ),
+ ArrowFormat::Field.new(
+ "count",
+
ArrowFormat::UInt32Type.singleton,
+ ),
+ ])
+ end
+
+ test("records") do
+ record_batch = ArrowFormat::RecordBatch.new(
+ @schema,
+ [
+ {visible: true, count: 1},
+ nil,
+ [false, 3],
+ ],
+ )
+ assert_equal(@schema, record_batch.schema)
+ assert_equal(ArrowFormat::BooleanArray,
+ record_batch.find_column("visible").class)
+ assert_equal(ArrowFormat::UInt32Array,
+ record_batch.find_column("count").class)
+ assert_equal([
+ {"visible" => true, "count" => 1},
+ {"visible" => nil, "count" => nil},
+ {"visible" => false, "count" => 3},
Review Comment:
Could you align `"count"` or use one space before `"count"`?
```suggestion
{"visible" => true, "count" => 1},
{"visible" => nil, "count" => nil},
{"visible" => false, "count" => 3},
```
--
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]
