Skip to content

Commit

Permalink
stats: fix handling of NaN in min and max
Browse files Browse the repository at this point in the history
The `cmp::min` and `cmp::max` functions are not correct with partially
ordered values. #12712
  • Loading branch information
thestinger committed Mar 6, 2014
1 parent 5973b0c commit a871068
Showing 1 changed file with 11 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/libextra/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#[allow(missing_doc)];

use std::cmp;
use std::hash::Hash;
use std::io;
use std::mem;
Expand Down Expand Up @@ -203,12 +202,12 @@ impl<'a> Stats for &'a [f64] {

fn min(self) -> f64 {
assert!(self.len() != 0);
self.iter().fold(self[0], |p,q| cmp::min(p, *q))
self.iter().fold(self[0], |p, q| p.min(*q))
}

fn max(self) -> f64 {
assert!(self.len() != 0);
self.iter().fold(self[0], |p,q| cmp::max(p, *q))
self.iter().fold(self[0], |p, q| p.max(*q))
}

fn mean(self) -> f64 {
Expand Down Expand Up @@ -442,6 +441,7 @@ mod tests {
use stats::write_boxplot;
use std::io;
use std::str;
use std::f64;

macro_rules! assert_approx_eq(
($a:expr, $b:expr) => ({
Expand Down Expand Up @@ -481,6 +481,14 @@ mod tests {
assert_eq!(summ.iqr, summ2.iqr);
}

#[test]
fn test_min_max_nan() {
let xs = &[1.0, 2.0, f64::NAN, 3.0, 4.0];
let summary = Summary::new(xs);
assert_eq!(summary.min, 1.0);
assert_eq!(summary.max, 4.0);
}

#[test]
fn test_norm2() {
let val = &[
Expand Down

5 comments on commit a871068

@bors
Copy link
Contributor

@bors bors commented on a871068 Mar 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on a871068 Mar 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/min_max = a871068 into auto

@bors
Copy link
Contributor

@bors bors commented on a871068 Mar 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/min_max = a871068 merged ok, testing candidate = 67c5d79

@bors
Copy link
Contributor

@bors bors commented on a871068 Mar 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on a871068 Mar 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 67c5d79

Please sign in to comment.