Copilot commented on code in PR #49621:
URL: https://github.com/apache/arrow/pull/49621#discussion_r3013570897
##########
ruby/red-arrow-format/lib/arrow-format/dictionary.rb:
##########
@@ -0,0 +1,28 @@
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
Review Comment:
The Apache license header in this new file appears to be truncated/missing
the standard opening line (e.g., "Licensed to the Apache Software Foundation
(ASF) under one..."). Please update it to match the canonical header used in
the other files in this directory to avoid license/compliance issues.
##########
ruby/red-arrow-format/lib/arrow-format/readable.rb:
##########
@@ -34,12 +34,14 @@ def read_custom_metadata(fb_custom_metadata)
metadata
end
- def read_schema(fb_schema)
+ def read_schema(fb_schema, fb_message_custom_metadata=nil)
fields = fb_schema.fields.collect do |fb_field|
read_field(fb_field)
end
+ message_metadta = read_custom_metadata(fb_message_custom_metadata)
Schema.new(fields,
- metadata: read_custom_metadata(fb_schema.custom_metadata))
+ metadata: read_custom_metadata(fb_schema.custom_metadata),
+ message_metadata: message_metadta)
Review Comment:
In `read_schema`, the local variable is misspelled as `message_metadta`.
Even though it currently works, the typo makes the code harder to read and easy
to propagate; rename it to `message_metadata` for consistency with the keyword
argument and other usages.
##########
ruby/red-arrow-format/test/test-writer.rb:
##########
@@ -1002,7 +1058,9 @@ def build_record_batches(red_arrow_value_type, values1,
values2)
raw_dictionary_more = raw_dictionary | values2.uniq
red_arrow_dictionary_more =
red_arrow_value_type.build_array(raw_dictionary_more)
- dictionary_more = convert_array(red_arrow_dictionary_more)
+ dictionary_array_more = convert_array(red_arrow_dictionary_more)
+ dictionary_more = ArrowFormat::Dictionary.new(dictionary_id + 1,
Review Comment:
In the non-`chunked_dictionaries?` branch, `dictionary_more` is constructed
with `dictionary_id + 1` even though the field's `DictionaryType` was created
with `dictionary_id` (1). The dictionary batch id should match the schema's
dictionary encoding id; consider using `dictionary_id` here to keep the test
data consistent with the Arrow IPC model.
```suggestion
dictionary_more = ArrowFormat::Dictionary.new(dictionary_id,
```
--
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]
