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:
[email protected]
