You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/11/06 19:59:47 UTC

[GitHub] [arrow] carols10cents commented on a change in pull request #8590: ARROW-10042: [Rust] BufferData's equality should depend on its capacity; ArrayData's equality should not depend on its BufferDatas' capacities' equality

carols10cents commented on a change in pull request #8590:
URL: https://github.com/apache/arrow/pull/8590#discussion_r518973664



##########
File path: rust/arrow/src/array/data.rs
##########
@@ -211,57 +211,66 @@ impl ArrayData {
 
 impl PartialEq for ArrayData {
     fn eq(&self, other: &Self) -> bool {
-        assert_eq!(
-            self.data_type(),
-            other.data_type(),
-            "Data types not the same"
-        );
-        assert_eq!(self.len(), other.len(), "Lengths not the same");
-        // TODO: when adding tests for this, test that we can compare with arrays that have offsets
-        assert_eq!(self.offset(), other.offset(), "Offsets not the same");
-        assert_eq!(self.null_count(), other.null_count());
-        // compare buffers excluding padding
-        let self_buffers = self.buffers();
-        let other_buffers = other.buffers();
-        assert_eq!(self_buffers.len(), other_buffers.len());
-        self_buffers.iter().zip(other_buffers).for_each(|(s, o)| {
-            compare_buffer_regions(
-                s,
-                self.offset(), // TODO mul by data length
-                o,
-                other.offset(), // TODO mul by data len
-            );
-        });
-        // assert_eq!(self.buffers(), other.buffers());
-
-        assert_eq!(self.child_data(), other.child_data());
-        // null arrays can skip the null bitmap, thus only compare if there are no nulls
-        if self.null_count() != 0 || other.null_count() != 0 {
-            compare_buffer_regions(
+        self.data_type() == other.data_type()
+            && self.len() == other.len()
+            // TODO: when adding tests for this, test that we can compare with arrays that have
+            // offsets
+            && self.offset() == other.offset()
+            && compare_buffers(
+                    self.buffers(),
+                    self.offset(),
+                    other.buffers(),
+                    other.offset(),
+                )
+            && self.child_data() == other.child_data()
+            && self.null_count() == other.null_count()
+            // null arrays can skip the null bitmap, thus only compare if there are no nulls

Review comment:
       I'm pretty sure those are @nevi-me 's, I just kept them in there :)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org