WeightedIndex panics on NaN's

[wschella]

When calling rand::distributions::WeightedIndex::new with weights containing a NaN, the constructor panics with message Uniform::new called with low >= high. This makes a lot of sense of course, given that the total weight (and thus high) is also NaN.

Now, the documentation does not mention panicking, it returns errors for invalid parameters. All args are checked at runtime anyway (to see whether they are < 0) so it feels like it would make sense to similarly return an error here instead of panicking. It would requiring doing an explicit partial_cmp instead of simply <.
Either that or a small mention is made in the docs.

I could (and would) wip up a PR. But what is the rand teams preference for this?

Minimal reproducing example:

extern crate rand; // 0.7.3

use rand::distributions::WeightedIndex;

pub fn main() {
    WeightedIndex::new(vec![std::f64::NAN, 1.0]).unwrap();
}

Playground

[dhardy]

This should already have been changed in master I believe. If not, a PR is welcome.

[wschella]

The test below added on master fails due to panicking, and I don't see anything relevant in the git history. I'll make a PR returning a WeightedError::InvalidWeight.

    #[test]
    fn test_accepting_nan(){
        assert_eq!(
            WeightedIndex::new(&[std::f32::NAN, 0.5]).unwrap_err(),
            WeightedError::InvalidWeight,
        );
    }