[GitHub] [arrow] nevi-me commented on a change in pull request #7193: ARROW-7924: [Rust] Add sort for float types

[GitBox]

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



##########
File path: rust/arrow/src/compute/kernels/sort.rs
##########
@@ -283,6 +285,30 @@ mod tests {
             None,
             vec![3, 1, 4, 2, 0, 5],
         );
+        test_sort_to_indices_primitive_arrays::<Float32Type>(
+            vec![
+                None,
+                Some(-0.05),
+                Some(2.225),
+                Some(-1.01),
+                Some(-0.05),
+                None,
+            ],
+            None,
+            vec![3, 1, 4, 2, 0, 5],
+        );
+        test_sort_to_indices_primitive_arrays::<Float64Type>(
+            vec![
+                None,
+                Some(-0.05),
+                Some(2.225),
+                Some(-1.01),
+                Some(-0.05),
+                None,

Review comment:
       I see that we do get the failure if we have a `f{32|64}::NAN`. Handling 
`NAN` might complicate the integer case, so should I separate the float 
implementation?
   
   How do we treat NaN in general across the Rust implementation? 




----------------------------------------------------------------
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:
us...@infra.apache.org