[GitHub] [arrow] alamb commented on a change in pull request #8882: ARROW-10864: [Rust] Use standard ordering for floats

[GitBox <gi...@apache.org>]

alamb commented on a change in pull request #8882:
URL: https://github.com/apache/arrow/pull/8882#discussion_r552202489



##########
File path: rust/arrow/src/compute/kernels/sort.rs
##########
@@ -41,40 +41,33 @@ pub fn sort(values: &ArrayRef, options: Option<SortOptions>) -> Result<ArrayRef>
     take(values.as_ref(), &indices, None)
 }
 
-// partition indices into non-NaN and NaN
-fn partition_nan<T: ArrowPrimitiveType>(
-    array: &ArrayRef,
-    v: Vec<u32>,
-) -> (Vec<u32>, Vec<u32>) {
-    // partition by nan for float types
-    if matches!(T::DATA_TYPE, DataType::Float32) {
-        // T::Native has no `is_nan` and thus we need to downcast
-        let array = array
-            .as_any()
-            .downcast_ref::<Float32Array>()
-            .expect("Unable to downcast array");
-        let has_nan = v.iter().any(|index| array.value(*index as usize).is_nan());
-        if has_nan {
-            v.into_iter()
-                .partition(|index| !array.value(*index as usize).is_nan())
-        } else {
-            (v, vec![])
-        }
-    } else if matches!(T::DATA_TYPE, DataType::Float64) {
-        let array = array
-            .as_any()
-            .downcast_ref::<Float64Array>()
-            .expect("Unable to downcast array");
-        let has_nan = v.iter().any(|index| array.value(*index as usize).is_nan());
-        if has_nan {
-            v.into_iter()
-                .partition(|index| !array.value(*index as usize).is_nan())
-        } else {
-            (v, vec![])
-        }
-    } else {
-        unreachable!("Partition by nan is only applicable to float types")
-    }
+// sorts f32 in IEEE 754 total ordering

Review comment:
       It might be good here to provide a pointer to the original source implementation (e.g. https://doc.rust-lang.org/std/primitive.f64.html#method.total_cmp) and a TODO to change to use that API when is stabilized




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