westonpace opened a new issue, #5029:
URL: https://github.com/apache/arrow-rs/issues/5029

   **Describe the bug**
   The docs for 
[`arrow::compute::kernels::sort::sort`](https://docs.rs/arrow/latest/arrow/compute/kernels/sort/fn.sort.html)
 state:
   
   > Floats are sorted using IEEE 754 totalOrder
   
   The docs for 
[`arrow::compute::kernels::sort::sort_to_indices`](https://docs.rs/arrow/latest/arrow/compute/kernels/sort/fn.sort_to_indices.html)
 state:
   
   > For floating point arrays any NaN values are considered to be greater than 
any other non-null value
   
   These are different, which I suppose is technically ok (though not very 
helpful).  However, in practice, it seems that they both use total order.
   
   **To Reproduce**
   
   ```
   use std::sync::Arc;
   use arrow_array::{ArrayRef, Float32Array};
   pub fn main() {
       let pos_nan = f32::NAN;
       let neg_nan = -pos_nan;
       let zero = 0_f32;
   
       let vals: ArrayRef = Arc::new(Float32Array::from_iter_values(
           [zero, neg_nan, pos_nan].iter().copied(),
       ));
       // Prints NaN, 0.0, NaN -- matches the comment
       dbg!(arrow::compute::kernels::sort::sort(vals.as_ref(), None).unwrap());
       // Prints 1, 0, 2 -- matches total order, but not the comment
       dbg!(arrow::compute::kernels::sort::sort_to_indices(vals.as_ref(), None, 
None).unwrap());
   }
   ```
   
   **Expected behavior**
   My personal preference would be to update the comment to reflect that both 
sorting methods use total order.  If we can confirm that is the intention I can 
create a PR.
   
   **Additional context**
   N/A


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to