Update arrow version #8
Conversation
There are no known files to review.Currently, I support these languages:
|
|
@Swoorup looks like at least this has to be addressed first: apache/arrow-rs#5623 |
Thanks for pointing that out. |
🔄 New Commits Added
Reviewing these new changes. To re-review the whole PR, mention me and say
Summary of Changes
Identified Issues
Code Snippets for FixesFix for Issue 1File: fn finish(&mut self) -> arrow::array::ArrayRef {
let union_fields = match <#original_name as arrow_convert::field::ArrowField>::data_type().clone() {
arrow::datatypes::DataType::Union(fields, _) => fields,
_ => return Err(arrow::error::ArrowError::ComputeError("datatype is not a union".to_string())),
};
// rest of the code remains unchanged
}
fn finish_cloned(&self) -> arrow::array::ArrayRef {
let union_fields = match <#original_name as arrow_convert::field::ArrowField>::data_type().clone() {
arrow::datatypes::DataType::Union(fields, _) => fields,
_ => return Err(arrow::error::ArrowError::ComputeError("datatype is not a union".to_string())),
};
// rest of the code remains unchanged
}Fix for Issue 2File: let children = vec![#(
<#mutable_variant_array_types as arrow::array::ArrayBuilder>::finish(&mut self.#variant_names),
)*];
// Use `children` in both methodsGeneral Review
Yours, Gooroo.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
==========================================
- Coverage 93.19% 93.11% -0.09%
==========================================
Files 9 9
Lines 1764 1743 -21
==========================================
- Hits 1644 1623 -21
Misses 120 120 ☔ View full report in Codecov by Sentry. |
🔄 New Commits Added
Reviewing these new changes. To re-review the whole PR, mention me and say
Summary of Incoming Changes
Identified Issues
Code Snippets for FixesIssue 1: Best PracticesFile: arrow_convert_derive/src/derive_enum.rs quote! { Some(self.offsets.iter().cloned().collect::<arrow::buffer::ScalarBuffer<i32>>()) },Proposed Change: quote! { Some(arrow::buffer::ScalarBuffer::from_slice_ref(&self.offsets)) },Explanation: Directly using Issue 2: ReadabilityFile: arrow_convert_derive/src/derive_enum.rs // Removed field_type_ids as it is no longer necessary with the updated Arrow library.Explanation: Adding comments to explain the removal of General ReviewThe code quality and style are generally good. The most important issue is the potential overuse of intermediate collections, which can be optimized for better performance. Additionally, adding comments to explain significant changes would improve the readability and maintainability of the code. Yours, Gooroo.dev |
🔄 New Commits Added
Reviewing these new changes. To re-review the whole PR, mention me and say
Summary of Changes
Identified Issues
Code Snippets for FixesIssue 1: Proper Error HandlingLines: 377-380, 403-406 // Current code
.unwrap()
// Proposed change
.expect("Failed to create UnionArray")Issue 2: Add Comments for Removed FieldsLines: 243-245, 276-278 // Current code
// Removed code related to field_type_ids and num_variants
// Proposed change
// Removed field_type_ids and num_variants as they are no longer needed with the updated arrow version.General ReviewThe overall code quality is good, and the changes are clear and concise. The switch to Yours, Gooroo.dev |
🔄 New Commits Added
Reviewing these new changes. To re-review the whole PR, mention me and say
Summary of Incoming Changes
Identified Issues
Proposed FixesIssue 1: Best PracticesFile: Proposed Change: // The `field_type_ids` field and related logic have been removed as they are no longer necessary
// with the updated Arrow library version.Issue 2: ReadabilityFile: Proposed Change: // Simplified by removing `field_type_ids` which is no longer required.General ReviewThe code quality is generally good, and the style is consistent. The most important issues are the lack of documentation explaining the removal of Yours, Gooroo.dev |
Much better actually. Reduce code.