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/09/12 22:03:43 UTC

[GitHub] [arrow] nevi-me commented on a change in pull request #8173: ARROW-9980: [Rust] [Parquet] Fix clippy lints

nevi-me commented on a change in pull request #8173:
URL: https://github.com/apache/arrow/pull/8173#discussion_r487447527



##########
File path: rust/arrow/src/compute/kernels/aggregate.rs
##########
@@ -121,7 +121,7 @@ mod tests {
     #[test]
     fn test_primitive_array_float_sum() {
         let a = Float64Array::from(vec![1.1, 2.2, 3.3, 4.4, 5.5]);
-        assert_eq!(16.5, sum(&a).unwrap());
+        assert!(16.5 - sum(&a).unwrap() < f64::EPSILON);

Review comment:
       I like the approach of a macro, we can add it in a follow-up

##########
File path: rust/arrow/src/compute/kernels/aggregate.rs
##########
@@ -121,7 +121,7 @@ mod tests {
     #[test]
     fn test_primitive_array_float_sum() {
         let a = Float64Array::from(vec![1.1, 2.2, 3.3, 4.4, 5.5]);
-        assert_eq!(16.5, sum(&a).unwrap());
+        assert!(16.5 - sum(&a).unwrap() < f64::EPSILON);

Review comment:
       I like the approach of a macro, we can add it in a follow-up

##########
File path: rust/arrow/src/compute/kernels/aggregate.rs
##########
@@ -121,7 +121,7 @@ mod tests {
     #[test]
     fn test_primitive_array_float_sum() {
         let a = Float64Array::from(vec![1.1, 2.2, 3.3, 4.4, 5.5]);
-        assert_eq!(16.5, sum(&a).unwrap());
+        assert!(16.5 - sum(&a).unwrap() < f64::EPSILON);

Review comment:
       I like the approach of a macro, we can add it in a follow-up

##########
File path: rust/arrow/src/compute/kernels/aggregate.rs
##########
@@ -121,7 +121,7 @@ mod tests {
     #[test]
     fn test_primitive_array_float_sum() {
         let a = Float64Array::from(vec![1.1, 2.2, 3.3, 4.4, 5.5]);
-        assert_eq!(16.5, sum(&a).unwrap());
+        assert!(16.5 - sum(&a).unwrap() < f64::EPSILON);

Review comment:
       I like the approach of a macro, we can add it in a follow-up

##########
File path: rust/arrow/src/compute/kernels/aggregate.rs
##########
@@ -121,7 +121,7 @@ mod tests {
     #[test]
     fn test_primitive_array_float_sum() {
         let a = Float64Array::from(vec![1.1, 2.2, 3.3, 4.4, 5.5]);
-        assert_eq!(16.5, sum(&a).unwrap());
+        assert!(16.5 - sum(&a).unwrap() < f64::EPSILON);

Review comment:
       I can do this in the coming 2 weeks. I would agree with the approach of turning off the lint where it matters, which I think would be almost all cases.
   
   I had to merge this PR to bank the time (spent much of the Saturday working on this instead of a lot of other work)

##########
File path: rust/arrow/src/compute/kernels/aggregate.rs
##########
@@ -121,7 +121,7 @@ mod tests {
     #[test]
     fn test_primitive_array_float_sum() {
         let a = Float64Array::from(vec![1.1, 2.2, 3.3, 4.4, 5.5]);
-        assert_eq!(16.5, sum(&a).unwrap());
+        assert!(16.5 - sum(&a).unwrap() < f64::EPSILON);

Review comment:
       I can do this in the coming 2 weeks. I would agree with the approach of turning off the lint where it matters, which I think would be almost all cases.
   
   I had to merge this PR to bank the time (spent much of the Saturday working on this instead of a lot of other work) and because I already have 2 other PRs that I decided to prepare on top of the clippy lints




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