From bfb0db75b75768a3b54291dab67ec1ea90fcc13c Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 29 Jan 2019 09:00:46 +0000 Subject: [PATCH 01/29] Use Option as return type where things might fail --- src/histogram/grid.rs | 6 ++-- src/histogram/strategies.rs | 60 ++++++++++++++++++++++--------------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index 32a7161b..023e51a9 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -162,15 +162,15 @@ where /// /// [`Grid`]: struct.Grid.html /// [`strategy`]: strategies/index.html - pub fn from_array(array: &ArrayBase) -> Self + pub fn from_array(array: &ArrayBase) -> Option where S: Data, { - let bin_builders = array + let bin_builders: Option> = array .axis_iter(Axis(1)) .map(|data| B::from_array(&data)) .collect(); - Self { bin_builders } + bin_builders.map(|b| Self { bin_builders: b}) } /// Returns a [`Grid`] instance, built accordingly to the specified [`strategy`] diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index f669b35e..46503207 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -43,9 +43,10 @@ pub trait BinsBuildingStrategy /// that has learned the required parameter to build a collection of [`Bins`]. /// /// [`Bins`]: ../struct.Bins.html - fn from_array(array: &ArrayBase) -> Self + fn from_array(array: &ArrayBase) -> Option where - S: Data; + S: Data, + Self: std::marker::Sized; /// Returns a [`Bins`] instance, built accordingly to the parameters /// inferred from observations in [`from_array`]. @@ -181,8 +182,8 @@ impl BinsBuildingStrategy for Sqrt { type Elem = T; - /// **Panics** if the array is constant or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Self + /// Returns `None` if the array is constant or if `a.len()==0`. + fn from_array(a: &ArrayBase) -> Option where S: Data { @@ -192,7 +193,7 @@ impl BinsBuildingStrategy for Sqrt let max = a.max().unwrap().clone(); let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min, max); - Self { builder } + Some(Self { builder }) } fn build(&self) -> Bins { @@ -220,8 +221,8 @@ impl BinsBuildingStrategy for Rice { type Elem = T; - /// **Panics** if the array is constant or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Self + /// Returns `None` if the array is constant or if `a.len()==0`. + fn from_array(a: &ArrayBase) -> Option where S: Data { @@ -231,7 +232,7 @@ impl BinsBuildingStrategy for Rice let max = a.max().unwrap().clone(); let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min, max); - Self { builder } + Some(Self { builder }) } fn build(&self) -> Bins { @@ -259,8 +260,8 @@ impl BinsBuildingStrategy for Sturges { type Elem = T; - /// **Panics** if the array is constant or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Self + /// Returns `None` if the array is constant or if `a.len()==0`. + fn from_array(a: &ArrayBase) -> Option where S: Data { @@ -270,7 +271,7 @@ impl BinsBuildingStrategy for Sturges let max = a.max().unwrap().clone(); let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min, max); - Self { builder } + Some(Self { builder }) } fn build(&self) -> Bins { @@ -298,8 +299,8 @@ impl BinsBuildingStrategy for FreedmanDiaconis { type Elem = T; - /// **Panics** if `IQR==0` or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Self + /// Returns `None` if `IQR==0` or if `a.len()==0`. + fn from_array(a: &ArrayBase) -> Option where S: Data { @@ -314,7 +315,7 @@ impl BinsBuildingStrategy for FreedmanDiaconis let min = a_copy.min().unwrap().clone(); let max = a_copy.max().unwrap().clone(); let builder = EquiSpaced::new(bin_width, min, max); - Self { builder } + Some(Self { builder }) } fn build(&self) -> Bins { @@ -349,21 +350,32 @@ impl BinsBuildingStrategy for Auto { type Elem = T; - /// **Panics** if `IQR==0`, the array is constant, or `a.len()==0`. - fn from_array(a: &ArrayBase) -> Self + /// Returns `None` if `IQR==0` and the array is constant or `a.len()==0`. + fn from_array(a: &ArrayBase) -> Option where S: Data { let fd_builder = FreedmanDiaconis::from_array(&a); let sturges_builder = Sturges::from_array(&a); - let builder = { - if fd_builder.bin_width() > sturges_builder.bin_width() { - SturgesOrFD::Sturges(sturges_builder) - } else { - SturgesOrFD::FreedmanDiaconis(fd_builder) - } - }; - Self { builder } + match (fd_builder, sturges_builder) { + (None, None) => None, + (None, Some(sturges_builder)) => { + let builder = SturgesOrFD::Sturges(sturges_builder); + Some(Self { builder }) + }, + (Some(fd_builder), None) => { + let builder = SturgesOrFD::FreedmanDiaconis(fd_builder); + Some(Self { builder }) + }, + (Some(fd_builder), Some(sturges_builder)) => { + let builder = if fd_builder.bin_width() > sturges_builder.bin_width() { + SturgesOrFD::Sturges(sturges_builder) + } else { + SturgesOrFD::FreedmanDiaconis(fd_builder) + }; + Some(Self { builder }) + }, + } } fn build(&self) -> Bins { From 4057a72a4d3dbb5d9529289eaae153375d07a9d2 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 29 Jan 2019 09:08:29 +0000 Subject: [PATCH 02/29] Test suite aligned with docs --- src/histogram/strategies.rs | 42 ++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 46503207..da2b4bff 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -440,16 +440,14 @@ mod sqrt_tests { use super::*; use ndarray::array; - #[should_panic] #[test] fn constant_array_are_bad() { - Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]); + assert!(Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); } - #[should_panic] #[test] fn empty_arrays_cause_panic() { - let _: Sqrt = Sqrt::from_array(&array![]); + assert!(Sqrt::::from_array(&array![]).is_none()); } } @@ -458,16 +456,14 @@ mod rice_tests { use super::*; use ndarray::array; - #[should_panic] #[test] fn constant_array_are_bad() { - Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]); + assert!(Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); } - #[should_panic] #[test] fn empty_arrays_cause_panic() { - let _: Rice = Rice::from_array(&array![]); + assert!(Rice::::from_array(&array![]).is_none()); } } @@ -476,16 +472,14 @@ mod sturges_tests { use super::*; use ndarray::array; - #[should_panic] #[test] fn constant_array_are_bad() { - Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]); + assert!(Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); } - #[should_panic] #[test] fn empty_arrays_cause_panic() { - let _: Sturges = Sturges::from_array(&array![]); + assert!(Sturges::::from_array(&array![]).is_none()); } } @@ -494,22 +488,23 @@ mod fd_tests { use super::*; use ndarray::array; - #[should_panic] #[test] fn constant_array_are_bad() { - FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]); + assert!(FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); } - #[should_panic] #[test] fn zero_iqr_causes_panic() { - FreedmanDiaconis::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]); + assert!( + FreedmanDiaconis::from_array( + &array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20] + ).is_none() + ); } - #[should_panic] #[test] fn empty_arrays_cause_panic() { - let _: FreedmanDiaconis = FreedmanDiaconis::from_array(&array![]); + assert!(FreedmanDiaconis::::from_array(&array![]).is_none()); } } @@ -518,21 +513,20 @@ mod auto_tests { use super::*; use ndarray::array; - #[should_panic] #[test] fn constant_array_are_bad() { - Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]); + assert!(Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); } - #[should_panic] #[test] fn zero_iqr_causes_panic() { - Auto::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]); + assert!(Auto::from_array( + &array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20] + ).is_none()); } - #[should_panic] #[test] fn empty_arrays_cause_panic() { - let _: Auto = Auto::from_array(&array![]); + assert!(Auto::::from_array(&array![]).is_none()); } } From fa1eb34b8c2da4ebe085013cf8b442d16a208d9d Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:18:48 +0000 Subject: [PATCH 03/29] Equispaced does not panic anymore --- src/histogram/strategies.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index da2b4bff..190717f5 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -144,11 +144,14 @@ impl EquiSpaced where T: Ord + Clone + FromPrimitive + NumOps + Zero { - /// **Panics** if `bin_width<=0`. - fn new(bin_width: T, min: T, max: T) -> Self + /// Returns `None` if `bin_width<=0`. + fn new(bin_width: T, min: T, max: T) -> Option { - assert!(bin_width > T::zero()); - Self { bin_width, min, max } + if bin_width > T::zero() { + None + } else { + Some(Self { bin_width, min, max }) + } } fn build(&self) -> Bins { @@ -193,7 +196,7 @@ impl BinsBuildingStrategy for Sqrt let max = a.max().unwrap().clone(); let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min, max); - Some(Self { builder }) + builder.map(|b| Self {builder: b}) } fn build(&self) -> Bins { @@ -232,7 +235,7 @@ impl BinsBuildingStrategy for Rice let max = a.max().unwrap().clone(); let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min, max); - Some(Self { builder }) + builder.map(|b| Self {builder: b}) } fn build(&self) -> Bins { @@ -271,7 +274,7 @@ impl BinsBuildingStrategy for Sturges let max = a.max().unwrap().clone(); let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min, max); - Some(Self { builder }) + builder.map(|b| Self {builder: b}) } fn build(&self) -> Bins { @@ -315,7 +318,7 @@ impl BinsBuildingStrategy for FreedmanDiaconis let min = a_copy.min().unwrap().clone(); let max = a_copy.max().unwrap().clone(); let builder = EquiSpaced::new(bin_width, min, max); - Some(Self { builder }) + builder.map(|b| Self {builder: b}) } fn build(&self) -> Bins { From 669a33ff24a913f394502f13af948c14700c690c Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:28:19 +0000 Subject: [PATCH 04/29] Fixed some tests --- src/histogram/strategies.rs | 43 ++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 190717f5..9cc2e428 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -144,10 +144,10 @@ impl EquiSpaced where T: Ord + Clone + FromPrimitive + NumOps + Zero { - /// Returns `None` if `bin_width<=0`. + /// Returns `None` if `bin_width<=0` or `min` >= `max`. fn new(bin_width: T, min: T, max: T) -> Option { - if bin_width > T::zero() { + if (bin_width > T::zero()) | (min < max) { None } else { Some(Self { bin_width, min, max }) @@ -192,11 +192,14 @@ impl BinsBuildingStrategy for Sqrt { let n_elems = a.len(); let n_bins = (n_elems as f64).sqrt().round() as usize; - let min = a.min().unwrap().clone(); - let max = a.max().unwrap().clone(); - let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min, max); - builder.map(|b| Self {builder: b}) + match (a.min(), a.max()) { + (Some(min), Some(max)) => { + let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); + builder.map(|b| Self { builder: b }) + }, + _ => None, + } } fn build(&self) -> Bins { @@ -231,11 +234,14 @@ impl BinsBuildingStrategy for Rice { let n_elems = a.len(); let n_bins = (2. * (n_elems as f64).powf(1./3.)).round() as usize; - let min = a.min().unwrap().clone(); - let max = a.max().unwrap().clone(); - let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min, max); - builder.map(|b| Self {builder: b}) + match (a.min(), a.max()) { + (Some(min), Some(max)) => { + let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); + builder.map(|b| Self { builder: b }) + }, + _ => None, + } } fn build(&self) -> Bins { @@ -270,11 +276,14 @@ impl BinsBuildingStrategy for Sturges { let n_elems = a.len(); let n_bins = (n_elems as f64).log2().round() as usize + 1; - let min = a.min().unwrap().clone(); - let max = a.max().unwrap().clone(); - let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min, max); - builder.map(|b| Self {builder: b}) + match (a.min(), a.max()) { + (Some(min), Some(max)) => { + let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); + builder.map(|b| Self { builder: b }) + }, + _ => None, + } } fn build(&self) -> Bins { From 0e5eb6bfe67128a1e579293879eb84bff525d91a Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:31:25 +0000 Subject: [PATCH 05/29] Fixed FD tests --- src/histogram/strategies.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 9cc2e428..fb820924 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -317,6 +317,9 @@ impl BinsBuildingStrategy for FreedmanDiaconis S: Data { let n_points = a.len(); + if n_points == 0 { + return None + } let mut a_copy = a.to_owned(); let first_quartile = a_copy.quantile_mut::(0.25).unwrap(); @@ -324,10 +327,13 @@ impl BinsBuildingStrategy for FreedmanDiaconis let iqr = third_quartile - first_quartile; let bin_width = FreedmanDiaconis::compute_bin_width(n_points, iqr); - let min = a_copy.min().unwrap().clone(); - let max = a_copy.max().unwrap().clone(); - let builder = EquiSpaced::new(bin_width, min, max); - builder.map(|b| Self {builder: b}) + match (a.min(), a.max()) { + (Some(min), Some(max)) => { + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); + builder.map(|b| Self { builder: b }) + }, + _ => None, + } } fn build(&self) -> Bins { From 04cf0a7eb47e8bc7309c677a1867d878f7d8b405 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:34:53 +0000 Subject: [PATCH 06/29] Fixed wrong condition in IF --- src/histogram/strategies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index fb820924..4a5d8258 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -147,7 +147,7 @@ impl EquiSpaced /// Returns `None` if `bin_width<=0` or `min` >= `max`. fn new(bin_width: T, min: T, max: T) -> Option { - if (bin_width > T::zero()) | (min < max) { + if (bin_width <= T::zero()) | (min >= max) { None } else { Some(Self { bin_width, min, max }) From 4f429bcef37fd73155e4bff8e88864efbe479e30 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:37:50 +0000 Subject: [PATCH 07/29] Fixed wrong test --- src/histogram/strategies.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 4a5d8258..67cf2298 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -62,6 +62,7 @@ pub trait BinsBuildingStrategy fn n_bins(&self) -> usize; } +#[derive(Debug)] struct EquiSpaced { bin_width: T, min: T, @@ -74,6 +75,7 @@ struct EquiSpaced { /// Let `n` be the number of observations. Then /// /// `n_bins` = `sqrt(n)` +#[derive(Debug)] pub struct Sqrt { builder: EquiSpaced, } @@ -87,6 +89,7 @@ pub struct Sqrt { /// /// `n_bins` is only proportional to cube root of `n`. It tends to overestimate /// the `n_bins` and it does not take into account data variability. +#[derive(Debug)] pub struct Rice { builder: EquiSpaced, } @@ -99,6 +102,7 @@ pub struct Rice { /// is too conservative for larger, non-normal datasets. /// /// This is the default method in R’s hist method. +#[derive(Debug)] pub struct Sturges { builder: EquiSpaced, } @@ -117,10 +121,12 @@ pub struct Sturges { /// The [`IQR`] is very robust to outliers. /// /// [`IQR`]: https://en.wikipedia.org/wiki/Interquartile_range +#[derive(Debug)] pub struct FreedmanDiaconis { builder: EquiSpaced, } +#[derive(Debug)] enum SturgesOrFD { Sturges(Sturges), FreedmanDiaconis(FreedmanDiaconis), @@ -136,6 +142,7 @@ enum SturgesOrFD { /// /// [`Sturges`]: struct.Sturges.html /// [`FreedmanDiaconis`]: struct.FreedmanDiaconis.html +#[derive(Debug)] pub struct Auto { builder: SturgesOrFD, } @@ -537,10 +544,10 @@ mod auto_tests { } #[test] - fn zero_iqr_causes_panic() { + fn zero_iqr_is_handled_by_sturged() { assert!(Auto::from_array( &array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20] - ).is_none()); + ).is_some()); } #[test] From 4e74c483533c6564999901f21d7212c8c6ef67e4 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:39:10 +0000 Subject: [PATCH 08/29] Added new test for EquiSpaced and fixed old one --- src/histogram/strategies.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 67cf2298..49aeaa7e 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -453,10 +453,14 @@ where mod equispaced_tests { use super::*; - #[should_panic] #[test] fn bin_width_has_to_be_positive() { - EquiSpaced::new(0, 0, 200); + assert!(EquiSpaced::new(0, 0, 200).is_none()); + } + + #[test] + fn min_has_to_be_strictly_smaller_than_max() { + assert!(EquiSpaced::new(10, 0, 0).is_none()); } } From 56b7e458b0525c6564935ec79d0d42206e5cb8a4 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Wed, 30 Jan 2019 09:41:01 +0000 Subject: [PATCH 09/29] Fixed doc tests --- src/histogram/grid.rs | 2 +- src/histogram/histograms.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index 023e51a9..d7d5bed3 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -54,7 +54,7 @@ use ndarray::{ArrayBase, Data, Ix1, Ix2, Axis}; /// /// // The optimal grid layout is inferred from the data, /// // specifying a strategy (Auto in this case) -/// let grid = GridBuilder::>::from_array(&observations).build(); +/// let grid = GridBuilder::>::from_array(&observations).unwrap().build(); /// let expected_grid = Grid::from(vec![Bins::new(Edges::from(vec![1, 20, 39, 58, 77, 96, 115]))]); /// assert_eq!(grid, expected_grid); /// diff --git a/src/histogram/histograms.rs b/src/histogram/histograms.rs index 825aadb7..2821b002 100644 --- a/src/histogram/histograms.rs +++ b/src/histogram/histograms.rs @@ -123,7 +123,7 @@ pub trait HistogramExt /// [n64(-1.), n64(-0.5)], /// [n64(0.5), n64(-1.)] /// ]; - /// let grid = GridBuilder::>::from_array(&observations).build(); + /// let grid = GridBuilder::>::from_array(&observations).unwrap().build(); /// let expected_grid = Grid::from( /// vec![ /// Bins::new(Edges::from(vec![n64(-1.), n64(0.), n64(1.), n64(2.)])), From 12906fdeac377f927dab182f8037c19091b7ced0 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Sat, 2 Feb 2019 17:28:39 +0000 Subject: [PATCH 10/29] Fix docs. --- src/histogram/strategies.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 49aeaa7e..02b712bd 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -42,6 +42,9 @@ pub trait BinsBuildingStrategy /// Given some observations in a 1-dimensional array it returns a `BinsBuildingStrategy` /// that has learned the required parameter to build a collection of [`Bins`]. /// + /// It returns `None` if it is not possible to build a collection of [`Bins`] given + /// the observed data. + /// /// [`Bins`]: ../struct.Bins.html fn from_array(array: &ArrayBase) -> Option where From 9d1862f1bf02632709237493efc8eebd0d12b5ce Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Sat, 2 Feb 2019 17:29:04 +0000 Subject: [PATCH 11/29] Fix docs. --- src/histogram/strategies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 02b712bd..2c5866ac 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -43,7 +43,7 @@ pub trait BinsBuildingStrategy /// that has learned the required parameter to build a collection of [`Bins`]. /// /// It returns `None` if it is not possible to build a collection of [`Bins`] given - /// the observed data. + /// the observed data according to the chosen strategy. /// /// [`Bins`]: ../struct.Bins.html fn from_array(array: &ArrayBase) -> Option From 64789d6f25af009afe770fa270fd5c086f54bdde Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Sat, 2 Feb 2019 17:29:48 +0000 Subject: [PATCH 12/29] Fix docs. --- src/histogram/grid.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index d7d5bed3..eb891e5b 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -160,6 +160,9 @@ where /// it returns a `GridBuilder` instance that has learned the required parameter /// to build a [`Grid`] according to the specified [`strategy`]. /// + /// It returns `None` if it is not possible to build a [`Grid`] given + /// the observed data according to the chosen [`strategy`]. + /// /// [`Grid`]: struct.Grid.html /// [`strategy`]: strategies/index.html pub fn from_array(array: &ArrayBase) -> Option From fe150d1965e7fc2415b58cd4e0000fb45e64d3ab Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 22:20:36 +0000 Subject: [PATCH 13/29] Fmt --- src/correlation.rs | 96 ++++++++++++--------------- src/histogram/bins.rs | 35 ++++------ src/histogram/grid.rs | 31 +++++---- src/histogram/histograms.rs | 25 ++++--- src/histogram/mod.rs | 10 +-- src/histogram/strategies.rs | 111 ++++++++++++++++---------------- src/lib.rs | 17 +++-- src/quantile.rs | 9 ++- src/summary_statistics/means.rs | 47 ++++++++------ src/summary_statistics/mod.rs | 21 +++--- 10 files changed, 194 insertions(+), 208 deletions(-) diff --git a/src/correlation.rs b/src/correlation.rs index ec3a1030..200bbc45 100644 --- a/src/correlation.rs +++ b/src/correlation.rs @@ -131,14 +131,15 @@ where { let observation_axis = Axis(1); let n_observations = A::from_usize(self.len_of(observation_axis)).unwrap(); - let dof = - if ddof >= n_observations { - panic!("`ddof` needs to be strictly smaller than the \ - number of observations provided for each \ - random variable!") - } else { - n_observations - ddof - }; + let dof = if ddof >= n_observations { + panic!( + "`ddof` needs to be strictly smaller than the \ + number of observations provided for each \ + random variable!" + ) + } else { + n_observations - ddof + }; let mean = self.mean_axis(observation_axis); let denoised = self - &mean.insert_axis(observation_axis); let covariance = denoised.dot(&denoised.t()); @@ -156,7 +157,9 @@ where // observation per random variable (or no observations at all) let ddof = -A::one(); let cov = self.cov(ddof); - let std = self.std_axis(observation_axis, ddof).insert_axis(observation_axis); + let std = self + .std_axis(observation_axis, ddof) + .insert_axis(observation_axis); let std_matrix = std.dot(&std.t()); // element-wise division cov / std_matrix @@ -167,10 +170,10 @@ where mod cov_tests { use super::*; use ndarray::array; + use ndarray_rand::RandomExt; use quickcheck::quickcheck; use rand; use rand::distributions::Uniform; - use ndarray_rand::RandomExt; quickcheck! { fn constant_random_variables_have_zero_covariance_matrix(value: f64) -> bool { @@ -200,10 +203,7 @@ mod cov_tests { fn test_invalid_ddof() { let n_random_variables = 3; let n_observations = 4; - let a = Array::random( - (n_random_variables, n_observations), - Uniform::new(0., 10.) - ); + let a = Array::random((n_random_variables, n_observations), Uniform::new(0., 10.)); let invalid_ddof = (n_observations as f64) + rand::random::().abs(); a.cov(invalid_ddof); } @@ -235,45 +235,36 @@ mod cov_tests { #[test] fn test_covariance_for_random_array() { let a = array![ - [ 0.72009497, 0.12568055, 0.55705966, 0.5959984 , 0.69471457], - [ 0.56717131, 0.47619486, 0.21526298, 0.88915366, 0.91971245], - [ 0.59044195, 0.10720363, 0.76573717, 0.54693675, 0.95923036], - [ 0.24102952, 0.131347, 0.11118028, 0.21451351, 0.30515539], - [ 0.26952473, 0.93079841, 0.8080893 , 0.42814155, 0.24642258] + [0.72009497, 0.12568055, 0.55705966, 0.5959984, 0.69471457], + [0.56717131, 0.47619486, 0.21526298, 0.88915366, 0.91971245], + [0.59044195, 0.10720363, 0.76573717, 0.54693675, 0.95923036], + [0.24102952, 0.131347, 0.11118028, 0.21451351, 0.30515539], + [0.26952473, 0.93079841, 0.8080893, 0.42814155, 0.24642258] ]; let numpy_covariance = array![ - [ 0.05786248, 0.02614063, 0.06446215, 0.01285105, -0.06443992], - [ 0.02614063, 0.08733569, 0.02436933, 0.01977437, -0.06715555], - [ 0.06446215, 0.02436933, 0.10052129, 0.01393589, -0.06129912], - [ 0.01285105, 0.01977437, 0.01393589, 0.00638795, -0.02355557], - [-0.06443992, -0.06715555, -0.06129912, -0.02355557, 0.09909855] + [0.05786248, 0.02614063, 0.06446215, 0.01285105, -0.06443992], + [0.02614063, 0.08733569, 0.02436933, 0.01977437, -0.06715555], + [0.06446215, 0.02436933, 0.10052129, 0.01393589, -0.06129912], + [0.01285105, 0.01977437, 0.01393589, 0.00638795, -0.02355557], + [ + -0.06443992, + -0.06715555, + -0.06129912, + -0.02355557, + 0.09909855 + ] ]; assert_eq!(a.ndim(), 2); - assert!( - a.cov(1.).all_close( - &numpy_covariance, - 1e-8 - ) - ); + assert!(a.cov(1.).all_close(&numpy_covariance, 1e-8)); } #[test] #[should_panic] // We lose precision, hence the failing assert fn test_covariance_for_badly_conditioned_array() { - let a: Array2 = array![ - [ 1e12 + 1., 1e12 - 1.], - [ 1e-6 + 1e-12, 1e-6 - 1e-12], - ]; - let expected_covariance = array![ - [2., 2e-12], [2e-12, 2e-24] - ]; - assert!( - a.cov(1.).all_close( - &expected_covariance, - 1e-24 - ) - ); + let a: Array2 = array![[1e12 + 1., 1e12 - 1.], [1e-6 + 1e-12, 1e-6 - 1e-12],]; + let expected_covariance = array![[2., 2e-12], [2e-12, 2e-24]]; + assert!(a.cov(1.).all_close(&expected_covariance, 1e-24)); } } @@ -281,9 +272,9 @@ mod cov_tests { mod pearson_correlation_tests { use super::*; use ndarray::array; + use ndarray_rand::RandomExt; use quickcheck::quickcheck; use rand::distributions::Uniform; - use ndarray_rand::RandomExt; quickcheck! { fn output_matrix_is_symmetric(bound: f64) -> bool { @@ -337,19 +328,14 @@ mod pearson_correlation_tests { [0.26979716, 0.20887228, 0.95454999, 0.96290785] ]; let numpy_corrcoeff = array![ - [ 1. , 0.38089376, 0.08122504, -0.59931623, 0.1365648 ], - [ 0.38089376, 1. , 0.80918429, -0.52615195, 0.38954398], - [ 0.08122504, 0.80918429, 1. , 0.07134906, -0.17324776], - [-0.59931623, -0.52615195, 0.07134906, 1. , -0.8743213 ], - [ 0.1365648 , 0.38954398, -0.17324776, -0.8743213 , 1. ] + [1., 0.38089376, 0.08122504, -0.59931623, 0.1365648], + [0.38089376, 1., 0.80918429, -0.52615195, 0.38954398], + [0.08122504, 0.80918429, 1., 0.07134906, -0.17324776], + [-0.59931623, -0.52615195, 0.07134906, 1., -0.8743213], + [0.1365648, 0.38954398, -0.17324776, -0.8743213, 1.] ]; assert_eq!(a.ndim(), 2); - assert!( - a.pearson_correlation().all_close( - &numpy_corrcoeff, - 1e-7 - ) - ); + assert!(a.pearson_correlation().all_close(&numpy_corrcoeff, 1e-7)); } } diff --git a/src/histogram/bins.rs b/src/histogram/bins.rs index e0c3a1ea..3a83eae9 100644 --- a/src/histogram/bins.rs +++ b/src/histogram/bins.rs @@ -33,7 +33,6 @@ pub struct Edges { } impl From> for Edges { - /// Get an `Edges` instance from a `Vec`: /// the vector will be sorted in increasing order /// using an unstable sorting algorithm and duplicates @@ -89,7 +88,7 @@ impl From> for Edges { } } -impl Index for Edges{ +impl Index for Edges { type Output = A; /// Get the `i`-th edge. @@ -182,13 +181,11 @@ impl Edges { match self.edges.binary_search(value) { Ok(i) if i == n_edges - 1 => None, Ok(i) => Some((i, i + 1)), - Err(i) => { - match i { - 0 => None, - j if j == n_edges => None, - j => Some((j - 1, j)), - } - } + Err(i) => match i { + 0 => None, + j if j == n_edges => None, + j => Some((j - 1, j)), + }, } } @@ -309,18 +306,14 @@ impl Bins { /// ); /// ``` pub fn range_of(&self, value: &A) -> Option> - where - A: Clone, + where + A: Clone, { let edges_indexes = self.edges.indices_of(value); - edges_indexes.map( - |(left, right)| { - Range { - start: self.edges[left].clone(), - end: self.edges[right].clone(), - } - } - ) + edges_indexes.map(|(left, right)| Range { + start: self.edges[left].clone(), + end: self.edges[right].clone(), + }) } /// Get the `i`-th bin. @@ -341,7 +334,7 @@ impl Bins { /// ); /// ``` pub fn index(&self, index: usize) -> Range - where + where A: Clone, { // It was not possible to implement this functionality @@ -350,7 +343,7 @@ impl Bins { // Index, in fact, forces you to return a reference. Range { start: self.edges[index].clone(), - end: self.edges[index+1].clone(), + end: self.edges[index + 1].clone(), } } } diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index eb891e5b..7785b776 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -1,8 +1,8 @@ use super::bins::Bins; use super::strategies::BinsBuildingStrategy; -use std::ops::Range; use itertools::izip; -use ndarray::{ArrayBase, Data, Ix1, Ix2, Axis}; +use ndarray::{ArrayBase, Axis, Data, Ix1, Ix2}; +use std::ops::Range; /// A `Grid` is a partition of a rectangular region of an *n*-dimensional /// space—e.g. [*a*0, *b*0) × ⋯ × [*a**n*−1, @@ -72,7 +72,6 @@ pub struct Grid { } impl From>> for Grid { - /// Get a `Grid` instance from a `Vec>`. /// /// The `i`-th element in `Vec>` represents the 1-dimensional @@ -113,9 +112,14 @@ impl Grid { where S: Data, { - assert_eq!(point.len(), self.ndim(), - "Dimension mismatch: the point has {:?} dimensions, the grid \ - expected {:?} dimensions.", point.len(), self.ndim()); + assert_eq!( + point.len(), + self.ndim(), + "Dimension mismatch: the point has {:?} dimensions, the grid \ + expected {:?} dimensions.", + point.len(), + self.ndim() + ); point .iter() .zip(self.projections.iter()) @@ -132,9 +136,14 @@ impl Grid { /// **Panics** if at least one among `(i_0, ..., i_{n-1})` is out of bounds on the respective /// coordinate axis - i.e. if there exists `j` such that `i_j >= self.projections[j].len()`. pub fn index(&self, index: &[usize]) -> Vec> { - assert_eq!(index.len(), self.ndim(), - "Dimension mismatch: the index has {0:?} dimensions, the grid \ - expected {1:?} dimensions.", index.len(), self.ndim()); + assert_eq!( + index.len(), + self.ndim(), + "Dimension mismatch: the index has {0:?} dimensions, the grid \ + expected {1:?} dimensions.", + index.len(), + self.ndim() + ); izip!(&self.projections, index) .map(|(bins, &i)| bins.index(i)) .collect() @@ -167,13 +176,13 @@ where /// [`strategy`]: strategies/index.html pub fn from_array(array: &ArrayBase) -> Option where - S: Data, + S: Data, { let bin_builders: Option> = array .axis_iter(Axis(1)) .map(|data| B::from_array(&data)) .collect(); - bin_builders.map(|b| Self { bin_builders: b}) + bin_builders.map(|b| Self { bin_builders: b }) } /// Returns a [`Grid`] instance, built accordingly to the specified [`strategy`] diff --git a/src/histogram/histograms.rs b/src/histogram/histograms.rs index 2821b002..4eaeaad4 100644 --- a/src/histogram/histograms.rs +++ b/src/histogram/histograms.rs @@ -1,7 +1,7 @@ +use super::errors::BinNotFound; +use super::grid::Grid; use ndarray::prelude::*; use ndarray::Data; -use super::grid::Grid; -use super::errors::BinNotFound; /// Histogram data structure. pub struct Histogram { @@ -58,8 +58,8 @@ impl Histogram { Some(bin_index) => { self.counts[&*bin_index] += 1; Ok(()) - }, - None => Err(BinNotFound) + } + None => Err(BinNotFound), } } @@ -82,8 +82,8 @@ impl Histogram { /// Extension trait for `ArrayBase` providing methods to compute histograms. pub trait HistogramExt - where - S: Data, +where + S: Data, { /// Returns the [histogram](https://en.wikipedia.org/wiki/Histogram) /// for a 2-dimensional array of points `M`. @@ -145,17 +145,16 @@ pub trait HistogramExt /// # } /// ``` fn histogram(&self, grid: Grid) -> Histogram - where - A: Ord; + where + A: Ord; } impl HistogramExt for ArrayBase - where - S: Data, - A: Ord, +where + S: Data, + A: Ord, { - fn histogram(&self, grid: Grid) -> Histogram - { + fn histogram(&self, grid: Grid) -> Histogram { let mut histogram = Histogram::new(grid); for point in self.axis_iter(Axis(0)) { let _ = histogram.add_observation(&point); diff --git a/src/histogram/mod.rs b/src/histogram/mod.rs index 9176aee1..3acbf40a 100644 --- a/src/histogram/mod.rs +++ b/src/histogram/mod.rs @@ -1,10 +1,10 @@ //! Histogram functionalities. -pub use self::histograms::{Histogram, HistogramExt}; -pub use self::bins::{Edges, Bins}; +pub use self::bins::{Bins, Edges}; pub use self::grid::{Grid, GridBuilder}; +pub use self::histograms::{Histogram, HistogramExt}; -mod histograms; mod bins; -pub mod strategies; -mod grid; pub mod errors; +mod grid; +mod histograms; +pub mod strategies; diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 2c5866ac..d6c119f1 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -18,13 +18,12 @@ //! [`Grid`]: ../struct.Grid.html //! [`GridBuilder`]: ../struct.GridBuilder.html //! [`NumPy`]: https://docs.scipy.org/doc/numpy/reference/generated/numpy.histogram_bin_edges.html#numpy.histogram_bin_edges +use super::super::interpolate::Nearest; +use super::super::{Quantile1dExt, QuantileExt}; +use super::{Bins, Edges}; use ndarray::prelude::*; use ndarray::Data; use num_traits::{FromPrimitive, NumOps, Zero}; -use super::super::{QuantileExt, Quantile1dExt}; -use super::super::interpolate::Nearest; -use super::{Edges, Bins}; - /// A trait implemented by all strategies to build [`Bins`] /// with parameters inferred from observations. @@ -36,8 +35,7 @@ use super::{Edges, Bins}; /// [`Bins`]: ../struct.Bins.html /// [`Grid`]: ../struct.Grid.html /// [`GridBuilder`]: ../struct.GridBuilder.html -pub trait BinsBuildingStrategy -{ +pub trait BinsBuildingStrategy { type Elem: Ord; /// Given some observations in a 1-dimensional array it returns a `BinsBuildingStrategy` /// that has learned the required parameter to build a collection of [`Bins`]. @@ -48,7 +46,7 @@ pub trait BinsBuildingStrategy /// [`Bins`]: ../struct.Bins.html fn from_array(array: &ArrayBase) -> Option where - S: Data, + S: Data, Self: std::marker::Sized; /// Returns a [`Bins`] instance, built accordingly to the parameters @@ -151,24 +149,27 @@ pub struct Auto { } impl EquiSpaced - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { /// Returns `None` if `bin_width<=0` or `min` >= `max`. - fn new(bin_width: T, min: T, max: T) -> Option - { + fn new(bin_width: T, min: T, max: T) -> Option { if (bin_width <= T::zero()) | (min >= max) { None } else { - Some(Self { bin_width, min, max }) + Some(Self { + bin_width, + min, + max, + }) } } fn build(&self) -> Bins { let n_bins = self.n_bins(); let mut edges: Vec = vec![]; - for i in 0..(n_bins+1) { - let edge = self.min.clone() + T::from_usize(i).unwrap()*self.bin_width.clone(); + for i in 0..(n_bins + 1) { + let edge = self.min.clone() + T::from_usize(i).unwrap() * self.bin_width.clone(); edges.push(edge); } Bins::new(Edges::from(edges)) @@ -181,7 +182,7 @@ impl EquiSpaced max_edge = max_edge + self.bin_width.clone(); n_bins += 1; } - return n_bins + return n_bins; } fn bin_width(&self) -> T { @@ -190,15 +191,15 @@ impl EquiSpaced } impl BinsBuildingStrategy for Sqrt - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { type Elem = T; /// Returns `None` if the array is constant or if `a.len()==0`. fn from_array(a: &ArrayBase) -> Option where - S: Data + S: Data, { let n_elems = a.len(); let n_bins = (n_elems as f64).sqrt().round() as usize; @@ -207,7 +208,7 @@ impl BinsBuildingStrategy for Sqrt let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); builder.map(|b| Self { builder: b }) - }, + } _ => None, } } @@ -222,8 +223,8 @@ impl BinsBuildingStrategy for Sqrt } impl Sqrt - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { /// The bin width (or bin length) according to the fitted strategy. pub fn bin_width(&self) -> T { @@ -232,24 +233,24 @@ impl Sqrt } impl BinsBuildingStrategy for Rice - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { type Elem = T; /// Returns `None` if the array is constant or if `a.len()==0`. fn from_array(a: &ArrayBase) -> Option where - S: Data + S: Data, { let n_elems = a.len(); - let n_bins = (2. * (n_elems as f64).powf(1./3.)).round() as usize; + let n_bins = (2. * (n_elems as f64).powf(1. / 3.)).round() as usize; match (a.min(), a.max()) { (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); builder.map(|b| Self { builder: b }) - }, + } _ => None, } } @@ -264,8 +265,8 @@ impl BinsBuildingStrategy for Rice } impl Rice - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { /// The bin width (or bin length) according to the fitted strategy. pub fn bin_width(&self) -> T { @@ -274,15 +275,15 @@ impl Rice } impl BinsBuildingStrategy for Sturges - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { type Elem = T; /// Returns `None` if the array is constant or if `a.len()==0`. fn from_array(a: &ArrayBase) -> Option where - S: Data + S: Data, { let n_elems = a.len(); let n_bins = (n_elems as f64).log2().round() as usize + 1; @@ -291,7 +292,7 @@ impl BinsBuildingStrategy for Sturges let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); builder.map(|b| Self { builder: b }) - }, + } _ => None, } } @@ -306,8 +307,8 @@ impl BinsBuildingStrategy for Sturges } impl Sturges - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { /// The bin width (or bin length) according to the fitted strategy. pub fn bin_width(&self) -> T { @@ -316,19 +317,19 @@ impl Sturges } impl BinsBuildingStrategy for FreedmanDiaconis - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { type Elem = T; /// Returns `None` if `IQR==0` or if `a.len()==0`. fn from_array(a: &ArrayBase) -> Option where - S: Data + S: Data, { let n_points = a.len(); if n_points == 0 { - return None + return None; } let mut a_copy = a.to_owned(); @@ -341,7 +342,7 @@ impl BinsBuildingStrategy for FreedmanDiaconis (Some(min), Some(max)) => { let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); builder.map(|b| Self { builder: b }) - }, + } _ => None, } } @@ -356,11 +357,10 @@ impl BinsBuildingStrategy for FreedmanDiaconis } impl FreedmanDiaconis - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { - fn compute_bin_width(n_bins: usize, iqr: T) -> T - { + fn compute_bin_width(n_bins: usize, iqr: T) -> T { let denominator = (n_bins as f64).powf(1. / 3.); let bin_width = T::from_usize(2).unwrap() * iqr / T::from_f64(denominator).unwrap(); bin_width @@ -373,15 +373,15 @@ impl FreedmanDiaconis } impl BinsBuildingStrategy for Auto - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { type Elem = T; /// Returns `None` if `IQR==0` and the array is constant or `a.len()==0`. fn from_array(a: &ArrayBase) -> Option where - S: Data + S: Data, { let fd_builder = FreedmanDiaconis::from_array(&a); let sturges_builder = Sturges::from_array(&a); @@ -390,11 +390,11 @@ impl BinsBuildingStrategy for Auto (None, Some(sturges_builder)) => { let builder = SturgesOrFD::Sturges(sturges_builder); Some(Self { builder }) - }, + } (Some(fd_builder), None) => { let builder = SturgesOrFD::FreedmanDiaconis(fd_builder); Some(Self { builder }) - }, + } (Some(fd_builder), Some(sturges_builder)) => { let builder = if fd_builder.bin_width() > sturges_builder.bin_width() { SturgesOrFD::Sturges(sturges_builder) @@ -402,7 +402,7 @@ impl BinsBuildingStrategy for Auto SturgesOrFD::FreedmanDiaconis(fd_builder) }; Some(Self { builder }) - }, + } } } @@ -424,8 +424,8 @@ impl BinsBuildingStrategy for Auto } impl Auto - where - T: Ord + Clone + FromPrimitive + NumOps + Zero +where + T: Ord + Clone + FromPrimitive + NumOps + Zero, { /// The bin width (or bin length) according to the fitted strategy. pub fn bin_width(&self) -> T { @@ -528,9 +528,8 @@ mod fd_tests { #[test] fn zero_iqr_causes_panic() { assert!( - FreedmanDiaconis::from_array( - &array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20] - ).is_none() + FreedmanDiaconis::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]) + .is_none() ); } @@ -552,9 +551,7 @@ mod auto_tests { #[test] fn zero_iqr_is_handled_by_sturged() { - assert!(Auto::from_array( - &array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20] - ).is_some()); + assert!(Auto::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]).is_some()); } #[test] diff --git a/src/lib.rs b/src/lib.rs index 5d764a67..7fbcc94f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -23,30 +23,29 @@ //! [`NumPy`]: https://docs.scipy.org/doc/numpy-1.14.1/reference/routines.statistics.html //! [`StatsBase.jl`]: https://juliastats.github.io/StatsBase.jl/latest/ - +extern crate itertools; extern crate ndarray; extern crate noisy_float; extern crate num_traits; extern crate rand; -extern crate itertools; +#[cfg(test)] +extern crate approx; #[cfg(test)] extern crate ndarray_rand; #[cfg(test)] extern crate quickcheck; -#[cfg(test)] -extern crate approx; -pub use maybe_nan::{MaybeNan, MaybeNanExt}; -pub use quantile::{interpolate, QuantileExt, Quantile1dExt}; -pub use sort::Sort1dExt; pub use correlation::CorrelationExt; pub use histogram::HistogramExt; +pub use maybe_nan::{MaybeNan, MaybeNanExt}; +pub use quantile::{interpolate, Quantile1dExt, QuantileExt}; +pub use sort::Sort1dExt; pub use summary_statistics::SummaryStatisticsExt; +mod correlation; +pub mod histogram; mod maybe_nan; mod quantile; mod sort; -mod correlation; mod summary_statistics; -pub mod histogram; diff --git a/src/quantile.rs b/src/quantile.rs index 9188aa2f..c29ccba2 100644 --- a/src/quantile.rs +++ b/src/quantile.rs @@ -378,8 +378,8 @@ where /// Quantile methods for 1-D arrays. pub trait Quantile1dExt - where - S: Data, +where + S: Data, { /// Return the qth quantile of the data. /// @@ -418,8 +418,8 @@ pub trait Quantile1dExt } impl Quantile1dExt for ArrayBase - where - S: Data, +where + S: Data, { fn quantile_mut(&mut self, q: f64) -> Option where @@ -434,4 +434,3 @@ impl Quantile1dExt for ArrayBase } } } - diff --git a/src/summary_statistics/means.rs b/src/summary_statistics/means.rs index 97217c23..f1059efd 100644 --- a/src/summary_statistics/means.rs +++ b/src/summary_statistics/means.rs @@ -1,8 +1,7 @@ -use ndarray::{Data, Dimension, ArrayBase}; -use num_traits::{FromPrimitive, Float, Zero}; -use std::ops::{Add, Div}; use super::SummaryStatisticsExt; - +use ndarray::{ArrayBase, Data, Dimension}; +use num_traits::{Float, FromPrimitive, Zero}; +use std::ops::{Add, Div}; impl SummaryStatisticsExt for ArrayBase where @@ -11,7 +10,7 @@ where { fn mean(&self) -> Option where - A: Clone + FromPrimitive + Add + Div + Zero + A: Clone + FromPrimitive + Add + Div + Zero, { let n_elements = self.len(); if n_elements == 0 { @@ -31,8 +30,8 @@ where } fn geometric_mean(&self) -> Option - where - A: Float + FromPrimitive, + where + A: Float + FromPrimitive, { self.map(|x| x.ln()).mean().map(|x| x.exp()) } @@ -41,10 +40,10 @@ where #[cfg(test)] mod tests { use super::SummaryStatisticsExt; - use std::f64; use approx::abs_diff_eq; - use noisy_float::types::N64; use ndarray::{array, Array1}; + use noisy_float::types::N64; + use std::f64; #[test] fn test_means_with_nan_values() { @@ -73,16 +72,14 @@ mod tests { #[test] fn test_means_with_array_of_floats() { let a: Array1 = array![ - 0.99889651, 0.0150731 , 0.28492482, 0.83819218, 0.48413156, - 0.80710412, 0.41762936, 0.22879429, 0.43997224, 0.23831807, - 0.02416466, 0.6269962 , 0.47420614, 0.56275487, 0.78995021, - 0.16060581, 0.64635041, 0.34876609, 0.78543249, 0.19938356, - 0.34429457, 0.88072369, 0.17638164, 0.60819363, 0.250392 , - 0.69912532, 0.78855523, 0.79140914, 0.85084218, 0.31839879, - 0.63381769, 0.22421048, 0.70760302, 0.99216018, 0.80199153, - 0.19239188, 0.61356023, 0.31505352, 0.06120481, 0.66417377, - 0.63608897, 0.84959691, 0.43599069, 0.77867775, 0.88267754, - 0.83003623, 0.67016118, 0.67547638, 0.65220036, 0.68043427 + 0.99889651, 0.0150731, 0.28492482, 0.83819218, 0.48413156, 0.80710412, 0.41762936, + 0.22879429, 0.43997224, 0.23831807, 0.02416466, 0.6269962, 0.47420614, 0.56275487, + 0.78995021, 0.16060581, 0.64635041, 0.34876609, 0.78543249, 0.19938356, 0.34429457, + 0.88072369, 0.17638164, 0.60819363, 0.250392, 0.69912532, 0.78855523, 0.79140914, + 0.85084218, 0.31839879, 0.63381769, 0.22421048, 0.70760302, 0.99216018, 0.80199153, + 0.19239188, 0.61356023, 0.31505352, 0.06120481, 0.66417377, 0.63608897, 0.84959691, + 0.43599069, 0.77867775, 0.88267754, 0.83003623, 0.67016118, 0.67547638, 0.65220036, + 0.68043427 ]; // Computed using NumPy let expected_mean = 0.5475494059146699; @@ -92,7 +89,15 @@ mod tests { let expected_geometric_mean = 0.4345897639796527; abs_diff_eq!(a.mean().unwrap(), expected_mean, epsilon = f64::EPSILON); - abs_diff_eq!(a.harmonic_mean().unwrap(), expected_harmonic_mean, epsilon = f64::EPSILON); - abs_diff_eq!(a.geometric_mean().unwrap(), expected_geometric_mean, epsilon = f64::EPSILON); + abs_diff_eq!( + a.harmonic_mean().unwrap(), + expected_harmonic_mean, + epsilon = f64::EPSILON + ); + abs_diff_eq!( + a.geometric_mean().unwrap(), + expected_geometric_mean, + epsilon = f64::EPSILON + ); } } diff --git a/src/summary_statistics/mod.rs b/src/summary_statistics/mod.rs index ae05e709..6aca865f 100644 --- a/src/summary_statistics/mod.rs +++ b/src/summary_statistics/mod.rs @@ -1,14 +1,14 @@ //! Summary statistics (e.g. mean, variance, etc.). use ndarray::{Data, Dimension}; -use num_traits::{FromPrimitive, Float, Zero}; +use num_traits::{Float, FromPrimitive, Zero}; use std::ops::{Add, Div}; /// Extension trait for `ArrayBase` providing methods /// to compute several summary statistics (e.g. mean, variance, etc.). pub trait SummaryStatisticsExt - where - S: Data, - D: Dimension, +where + S: Data, + D: Dimension, { /// Returns the [`arithmetic mean`] x̅ of all elements in the array: /// @@ -24,8 +24,8 @@ pub trait SummaryStatisticsExt /// /// [`arithmetic mean`]: https://en.wikipedia.org/wiki/Arithmetic_mean fn mean(&self) -> Option - where - A: Clone + FromPrimitive + Add + Div + Zero; + where + A: Clone + FromPrimitive + Add + Div + Zero; /// Returns the [`harmonic mean`] `HM(X)` of all elements in the array: /// @@ -41,8 +41,8 @@ pub trait SummaryStatisticsExt /// /// [`harmonic mean`]: https://en.wikipedia.org/wiki/Harmonic_mean fn harmonic_mean(&self) -> Option - where - A: Float + FromPrimitive; + where + A: Float + FromPrimitive; /// Returns the [`geometric mean`] `GM(X)` of all elements in the array: /// @@ -58,9 +58,8 @@ pub trait SummaryStatisticsExt /// /// [`geometric mean`]: https://en.wikipedia.org/wiki/Geometric_mean fn geometric_mean(&self) -> Option - where - A: Float + FromPrimitive; - + where + A: Float + FromPrimitive; } mod means; From b28c35aab40d98ca8536a3d34eb3a6350d3ddcd3 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 22:58:17 +0000 Subject: [PATCH 14/29] Create StrategyError --- src/histogram/errors.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/histogram/errors.rs b/src/histogram/errors.rs index 7afaea1f..0588cb48 100644 --- a/src/histogram/errors.rs +++ b/src/histogram/errors.rs @@ -15,9 +15,19 @@ impl error::Error for BinNotFound { fn description(&self) -> &str { "No bin has been found." } +} + +#[derive(Debug, Clone)] +pub struct StrategyError; + +impl fmt::Display for StrategyError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "The strategy failed to determine a non-zero bin width.") + } +} - fn cause(&self) -> Option<&error::Error> { - // Generic error, underlying cause isn't tracked. - None +impl error::Error for StrategyError{ + fn description(&self) -> &str { + "The strategy failed to determine a non-zero bin width." } } From c06f382884296d97da30ab9a0e0b11fb4b77eb2d Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 22:58:28 +0000 Subject: [PATCH 15/29] Fmt --- src/histogram/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram/errors.rs b/src/histogram/errors.rs index 0588cb48..08f08ac9 100644 --- a/src/histogram/errors.rs +++ b/src/histogram/errors.rs @@ -26,7 +26,7 @@ impl fmt::Display for StrategyError { } } -impl error::Error for StrategyError{ +impl error::Error for StrategyError { fn description(&self) -> &str { "The strategy failed to determine a non-zero bin width." } From 4a24f5ad6a7a8cf6acabcc283fa9aab101964f78 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:04:43 +0000 Subject: [PATCH 16/29] Return Result. Fix Equispaced, Sqrt and Rice --- src/histogram/strategies.rs | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index d6c119f1..0c5bf6ea 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -21,6 +21,7 @@ use super::super::interpolate::Nearest; use super::super::{Quantile1dExt, QuantileExt}; use super::{Bins, Edges}; +use super::errors::StrategyError; use ndarray::prelude::*; use ndarray::Data; use num_traits::{FromPrimitive, NumOps, Zero}; @@ -44,7 +45,7 @@ pub trait BinsBuildingStrategy { /// the observed data according to the chosen strategy. /// /// [`Bins`]: ../struct.Bins.html - fn from_array(array: &ArrayBase) -> Option + fn from_array(array: &ArrayBase) -> Result, StrategyError> where S: Data, Self: std::marker::Sized; @@ -152,12 +153,13 @@ impl EquiSpaced where T: Ord + Clone + FromPrimitive + NumOps + Zero, { - /// Returns `None` if `bin_width<=0` or `min` >= `max`. - fn new(bin_width: T, min: T, max: T) -> Option { + /// Returns `Err(StrategyError)` if `bin_width<=0` or `min` >= `max`. + /// Returns `Ok(Self)` otherwise. + fn new(bin_width: T, min: T, max: T) -> Result { if (bin_width <= T::zero()) | (min >= max) { - None + Err(StrategyError) } else { - Some(Self { + Ok(Self { bin_width, min, max, @@ -196,8 +198,10 @@ where { type Elem = T; - /// Returns `None` if the array is constant or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Option + /// Returns `Err(StrategyError)` if the array is constant. + /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Ok(Self)` otherwise. + fn from_array(a: &ArrayBase) -> Result, StrategyError> where S: Data, { @@ -206,10 +210,10 @@ where match (a.min(), a.max()) { (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); - builder.map(|b| Self { builder: b }) + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Self { builder } } - _ => None, + _ => Ok(None), } } @@ -238,8 +242,10 @@ where { type Elem = T; - /// Returns `None` if the array is constant or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Option + /// Returns `Err(StrategyError)` if the array is constant. + /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Ok(Self)` otherwise. + fn from_array(a: &ArrayBase) -> Result, StrategyError> where S: Data, { @@ -248,10 +254,10 @@ where match (a.min(), a.max()) { (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); - builder.map(|b| Self { builder: b }) + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Self { builder } } - _ => None, + _ => Ok(None), } } From f708a175ec1b90c12d725b03923b95655d832f42 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:05:26 +0000 Subject: [PATCH 17/29] Fix Rice --- src/histogram/strategies.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 0c5bf6ea..4d4f4d5a 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -286,8 +286,10 @@ where { type Elem = T; - /// Returns `None` if the array is constant or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Option + /// Returns `Err(StrategyError)` if the array is constant. + /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Ok(Self)` otherwise. + fn from_array(a: &ArrayBase) -> Result, StrategyError> where S: Data, { @@ -296,8 +298,8 @@ where match (a.min(), a.max()) { (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); - builder.map(|b| Self { builder: b }) + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Self { builder } } _ => None, } From 58788db55cf0adaa2648a73e9f25b6800998637a Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:06:20 +0000 Subject: [PATCH 18/29] Fixed Sturges --- src/histogram/strategies.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 4d4f4d5a..6bb0d707 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -330,8 +330,10 @@ where { type Elem = T; - /// Returns `None` if `IQR==0` or if `a.len()==0`. - fn from_array(a: &ArrayBase) -> Option + /// Returns `Err(StrategyError)` if `IQR==0`. + /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Ok(Self)` otherwise. + fn from_array(a: &ArrayBase) -> Result, StrategyError> where S: Data, { @@ -348,10 +350,10 @@ where let bin_width = FreedmanDiaconis::compute_bin_width(n_points, iqr); match (a.min(), a.max()) { (Some(min), Some(max)) => { - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone()); - builder.map(|b| Self { builder: b }) + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Self { builder } } - _ => None, + _ => Ok(None), } } From 3014f775324502045403685d6608c07a2ef3954f Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:12:29 +0000 Subject: [PATCH 19/29] Fix strategies --- src/histogram/strategies.rs | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 6bb0d707..6abcae5e 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -211,7 +211,7 @@ where (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Self { builder } + Ok(Some(Self { builder })) } _ => Ok(None), } @@ -255,7 +255,7 @@ where (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Self { builder } + Ok(Some(Self { builder })) } _ => Ok(None), } @@ -299,9 +299,9 @@ where (Some(min), Some(max)) => { let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Self { builder } + Ok(Some(Self { builder })) } - _ => None, + _ => Ok(None), } } @@ -339,7 +339,7 @@ where { let n_points = a.len(); if n_points == 0 { - return None; + return Ok(None); } let mut a_copy = a.to_owned(); @@ -351,7 +351,7 @@ where match (a.min(), a.max()) { (Some(min), Some(max)) => { let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Self { builder } + Ok(Some(Self { builder })) } _ => Ok(None), } @@ -388,30 +388,33 @@ where { type Elem = T; - /// Returns `None` if `IQR==0` and the array is constant or `a.len()==0`. - fn from_array(a: &ArrayBase) -> Option + /// Returns `Err(StrategyError)` if `IQR==0`. + /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Ok(Self)` otherwise. + fn from_array(a: &ArrayBase) -> Result, StrategyError> where S: Data, { let fd_builder = FreedmanDiaconis::from_array(&a); let sturges_builder = Sturges::from_array(&a); match (fd_builder, sturges_builder) { - (None, None) => None, - (None, Some(sturges_builder)) => { + (Err(_), Err(_)) => Err(StrategyError), + (Ok(None), Ok(None)) => Ok(None), + (Ok(None), Ok(Some(sturges_builder))) | (Err(_), Ok(Some(sturges_builder))) => { let builder = SturgesOrFD::Sturges(sturges_builder); - Some(Self { builder }) + Ok(Some(Self { builder })) } - (Some(fd_builder), None) => { + (Ok(Some(fd_builder)), Ok(None)) | (Ok(Some(fd_builder)), Err(_)) => { let builder = SturgesOrFD::FreedmanDiaconis(fd_builder); - Some(Self { builder }) + Ok(Some(Self { builder })) } - (Some(fd_builder), Some(sturges_builder)) => { + (Ok(Some(fd_builder)), Ok(Some(sturges_builder))) => { let builder = if fd_builder.bin_width() > sturges_builder.bin_width() { SturgesOrFD::Sturges(sturges_builder) } else { SturgesOrFD::FreedmanDiaconis(fd_builder) }; - Some(Self { builder }) + Ok(Some(Self { builder })) } } } From 17e5efc469db6925bf088bc9c057a61a5537ee7f Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:20:27 +0000 Subject: [PATCH 20/29] Fix match --- src/histogram/grid.rs | 7 ++++++- src/histogram/strategies.rs | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index 7785b776..c6237f06 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -180,7 +180,12 @@ where { let bin_builders: Option> = array .axis_iter(Axis(1)) - .map(|data| B::from_array(&data)) + .map(|data| { + match B::from_array(&data) { + Ok(Some(builder)) => Some(builder), + _ => None + } + }) .collect(); bin_builders.map(|b| Self { bin_builders: b }) } diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 6abcae5e..c5b740c9 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -398,8 +398,6 @@ where let fd_builder = FreedmanDiaconis::from_array(&a); let sturges_builder = Sturges::from_array(&a); match (fd_builder, sturges_builder) { - (Err(_), Err(_)) => Err(StrategyError), - (Ok(None), Ok(None)) => Ok(None), (Ok(None), Ok(Some(sturges_builder))) | (Err(_), Ok(Some(sturges_builder))) => { let builder = SturgesOrFD::Sturges(sturges_builder); Ok(Some(Self { builder })) @@ -416,6 +414,10 @@ where }; Ok(Some(Self { builder })) } + (Err(_), Err(_)) => Err(StrategyError), + (Ok(None), Ok(None)) => Ok(None), + // Unreachable + (Err(_), Ok(None)) | (Ok(None), Err(_)) => Err(StrategyError) } } From 63abed5613ec5222c6d6b9e383c70d73e89cf0b2 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:25:40 +0000 Subject: [PATCH 21/29] Tests compile --- src/histogram/strategies.rs | 45 ++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index c5b740c9..e36a718d 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -473,12 +473,12 @@ mod equispaced_tests { #[test] fn bin_width_has_to_be_positive() { - assert!(EquiSpaced::new(0, 0, 200).is_none()); + assert!(EquiSpaced::new(0, 0, 200).is_err()); } #[test] fn min_has_to_be_strictly_smaller_than_max() { - assert!(EquiSpaced::new(10, 0, 0).is_none()); + assert!(EquiSpaced::new(10, 0, 0).is_err()); } } @@ -489,12 +489,13 @@ mod sqrt_tests { #[test] fn constant_array_are_bad() { - assert!(Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); + assert!(Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); } #[test] - fn empty_arrays_cause_panic() { - assert!(Sqrt::::from_array(&array![]).is_none()); + fn empty_arrays_are_bad() -> Result<(), StrategyError> { + assert!(Sqrt::::from_array(&array![])?.is_none()); + Ok(()) } } @@ -505,12 +506,13 @@ mod rice_tests { #[test] fn constant_array_are_bad() { - assert!(Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); + assert!(Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); } #[test] - fn empty_arrays_cause_panic() { - assert!(Rice::::from_array(&array![]).is_none()); + fn empty_arrays_are_bad() -> Result<(), StrategyError> { + assert!(Rice::::from_array(&array![])?.is_none()); + Ok(()) } } @@ -521,12 +523,13 @@ mod sturges_tests { #[test] fn constant_array_are_bad() { - assert!(Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); + assert!(Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); } #[test] - fn empty_arrays_cause_panic() { - assert!(Sturges::::from_array(&array![]).is_none()); + fn empty_arrays_are_bad() -> Result<(), StrategyError> { + assert!(Sturges::::from_array(&array![])?.is_none()); + Ok(()) } } @@ -537,20 +540,21 @@ mod fd_tests { #[test] fn constant_array_are_bad() { - assert!(FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); + assert!(FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); } #[test] - fn zero_iqr_causes_panic() { + fn zero_iqr_is_bad(){ assert!( FreedmanDiaconis::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]) - .is_none() + .is_err() ); } #[test] - fn empty_arrays_cause_panic() { - assert!(FreedmanDiaconis::::from_array(&array![]).is_none()); + fn empty_arrays_are_bad() -> Result<(), StrategyError> { + assert!(FreedmanDiaconis::::from_array(&array![])?.is_none()); + Ok(()) } } @@ -561,16 +565,17 @@ mod auto_tests { #[test] fn constant_array_are_bad() { - assert!(Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_none()); + assert!(Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); } #[test] fn zero_iqr_is_handled_by_sturged() { - assert!(Auto::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]).is_some()); + assert!(Auto::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]).is_err()); } #[test] - fn empty_arrays_cause_panic() { - assert!(Auto::::from_array(&array![]).is_none()); + fn empty_arrays_cause_panic() -> Result<(), StrategyError> { + assert!(Auto::::from_array(&array![])?.is_none()); + Ok(()) } } From 4a4b489372885c51c64280ca3303aefe8e4e7ef4 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:26:14 +0000 Subject: [PATCH 22/29] Fix assertion --- src/histogram/strategies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index e36a718d..56d47594 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -570,7 +570,7 @@ mod auto_tests { #[test] fn zero_iqr_is_handled_by_sturged() { - assert!(Auto::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]).is_err()); + assert!(Auto::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]).is_ok()); } #[test] From f69288766d153f1c1b64dc51769d9755a0bce990 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Tue, 26 Mar 2019 23:29:21 +0000 Subject: [PATCH 23/29] Fmt --- src/histogram/grid.rs | 8 +++----- src/histogram/strategies.rs | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index c6237f06..06914d77 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -180,11 +180,9 @@ where { let bin_builders: Option> = array .axis_iter(Axis(1)) - .map(|data| { - match B::from_array(&data) { - Ok(Some(builder)) => Some(builder), - _ => None - } + .map(|data| match B::from_array(&data) { + Ok(Some(builder)) => Some(builder), + _ => None, }) .collect(); bin_builders.map(|b| Self { bin_builders: b }) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 56d47594..b2e1361b 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -20,8 +20,8 @@ //! [`NumPy`]: https://docs.scipy.org/doc/numpy/reference/generated/numpy.histogram_bin_edges.html#numpy.histogram_bin_edges use super::super::interpolate::Nearest; use super::super::{Quantile1dExt, QuantileExt}; -use super::{Bins, Edges}; use super::errors::StrategyError; +use super::{Bins, Edges}; use ndarray::prelude::*; use ndarray::Data; use num_traits::{FromPrimitive, NumOps, Zero}; @@ -417,7 +417,7 @@ where (Err(_), Err(_)) => Err(StrategyError), (Ok(None), Ok(None)) => Ok(None), // Unreachable - (Err(_), Ok(None)) | (Ok(None), Err(_)) => Err(StrategyError) + (Err(_), Ok(None)) | (Ok(None), Err(_)) => Err(StrategyError), } } @@ -544,7 +544,7 @@ mod fd_tests { } #[test] - fn zero_iqr_is_bad(){ + fn zero_iqr_is_bad() { assert!( FreedmanDiaconis::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]) .is_err() From a8ad4b18b254647fd39e9a0d8e59ed838c390b5e Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 17:30:49 -0400 Subject: [PATCH 24/29] Add more error types --- src/entropy.rs | 97 +++++++++---------- src/errors.rs | 92 +++++++++++++++++- src/histogram/errors.rs | 42 +++++++- src/histogram/grid.rs | 2 +- src/histogram/strategies.rs | 167 ++++++++++++++++---------------- src/quantile.rs | 97 ++++++++++--------- src/summary_statistics/means.rs | 24 ++--- src/summary_statistics/mod.rs | 13 +-- tests/quantile.rs | 33 ++++--- 9 files changed, 354 insertions(+), 213 deletions(-) diff --git a/src/entropy.rs b/src/entropy.rs index 8daac6d8..3841cb02 100644 --- a/src/entropy.rs +++ b/src/entropy.rs @@ -1,5 +1,5 @@ //! Information theory (e.g. entropy, KL divergence, etc.). -use crate::errors::ShapeMismatch; +use crate::errors::{EmptyInput, MultiInputError, ShapeMismatch}; use ndarray::{Array, ArrayBase, Data, Dimension, Zip}; use num_traits::Float; @@ -19,7 +19,7 @@ where /// i=1 /// ``` /// - /// If the array is empty, `None` is returned. + /// If the array is empty, `Err(EmptyInput)` is returned. /// /// **Panics** if `ln` of any element in the array panics (which can occur for negative values for some `A`). /// @@ -38,7 +38,7 @@ where /// /// [entropy]: https://en.wikipedia.org/wiki/Entropy_(information_theory) /// [Information Theory]: https://en.wikipedia.org/wiki/Information_theory - fn entropy(&self) -> Option + fn entropy(&self) -> Result where A: Float; @@ -53,8 +53,9 @@ where /// i=1 /// ``` /// - /// If the arrays are empty, Ok(`None`) is returned. - /// If the array shapes are not identical, `Err(ShapeMismatch)` is returned. + /// If the arrays are empty, `Err(MultiInputError::EmptyInput)` is returned. + /// If the array shapes are not identical, + /// `Err(MultiInputError::ShapeMismatch)` is returned. /// /// **Panics** if, for a pair of elements *(pᵢ, qᵢ)* from *p* and *q*, computing /// *ln(qᵢ/pᵢ)* is a panic cause for `A`. @@ -73,7 +74,7 @@ where /// /// [Kullback-Leibler divergence]: https://en.wikipedia.org/wiki/Kullback%E2%80%93Leibler_divergence /// [Information Theory]: https://en.wikipedia.org/wiki/Information_theory - fn kl_divergence(&self, q: &ArrayBase) -> Result, ShapeMismatch> + fn kl_divergence(&self, q: &ArrayBase) -> Result where S2: Data, A: Float; @@ -89,8 +90,9 @@ where /// i=1 /// ``` /// - /// If the arrays are empty, Ok(`None`) is returned. - /// If the array shapes are not identical, `Err(ShapeMismatch)` is returned. + /// If the arrays are empty, `Err(MultiInputError::EmptyInput)` is returned. + /// If the array shapes are not identical, + /// `Err(MultiInputError::ShapeMismatch)` is returned. /// /// **Panics** if any element in *q* is negative and taking the logarithm of a negative number /// is a panic cause for `A`. @@ -114,7 +116,7 @@ where /// [Information Theory]: https://en.wikipedia.org/wiki/Information_theory /// [optimization problems]: https://en.wikipedia.org/wiki/Cross-entropy_method /// [machine learning]: https://en.wikipedia.org/wiki/Cross_entropy#Cross-entropy_error_function_and_logistic_regression - fn cross_entropy(&self, q: &ArrayBase) -> Result, ShapeMismatch> + fn cross_entropy(&self, q: &ArrayBase) -> Result where S2: Data, A: Float; @@ -125,14 +127,14 @@ where S: Data, D: Dimension, { - fn entropy(&self) -> Option + fn entropy(&self) -> Result where A: Float, { if self.len() == 0 { - None + Err(EmptyInput) } else { - let entropy = self + let entropy = -self .mapv(|x| { if x == A::zero() { A::zero() @@ -141,23 +143,24 @@ where } }) .sum(); - Some(-entropy) + Ok(entropy) } } - fn kl_divergence(&self, q: &ArrayBase) -> Result, ShapeMismatch> + fn kl_divergence(&self, q: &ArrayBase) -> Result where A: Float, S2: Data, { if self.len() == 0 { - return Ok(None); + return Err(MultiInputError::EmptyInput); } if self.shape() != q.shape() { return Err(ShapeMismatch { first_shape: self.shape().to_vec(), second_shape: q.shape().to_vec(), - }); + } + .into()); } let mut temp = Array::zeros(self.raw_dim()); @@ -174,22 +177,23 @@ where } }); let kl_divergence = -temp.sum(); - Ok(Some(kl_divergence)) + Ok(kl_divergence) } - fn cross_entropy(&self, q: &ArrayBase) -> Result, ShapeMismatch> + fn cross_entropy(&self, q: &ArrayBase) -> Result where S2: Data, A: Float, { if self.len() == 0 { - return Ok(None); + return Err(MultiInputError::EmptyInput); } if self.shape() != q.shape() { return Err(ShapeMismatch { first_shape: self.shape().to_vec(), second_shape: q.shape().to_vec(), - }); + } + .into()); } let mut temp = Array::zeros(self.raw_dim()); @@ -206,7 +210,7 @@ where } }); let cross_entropy = -temp.sum(); - Ok(Some(cross_entropy)) + Ok(cross_entropy) } } @@ -214,7 +218,7 @@ where mod tests { use super::EntropyExt; use approx::assert_abs_diff_eq; - use errors::ShapeMismatch; + use errors::{EmptyInput, MultiInputError}; use ndarray::{array, Array1}; use noisy_float::types::n64; use std::f64; @@ -228,7 +232,7 @@ mod tests { #[test] fn test_entropy_with_empty_array_of_floats() { let a: Array1 = array![]; - assert!(a.entropy().is_none()); + assert_eq!(a.entropy(), Err(EmptyInput)); } #[test] @@ -251,13 +255,13 @@ mod tests { } #[test] - fn test_cross_entropy_and_kl_with_nan_values() -> Result<(), ShapeMismatch> { + fn test_cross_entropy_and_kl_with_nan_values() -> Result<(), MultiInputError> { let a = array![f64::NAN, 1.]; let b = array![2., 1.]; - assert!(a.cross_entropy(&b)?.unwrap().is_nan()); - assert!(b.cross_entropy(&a)?.unwrap().is_nan()); - assert!(a.kl_divergence(&b)?.unwrap().is_nan()); - assert!(b.kl_divergence(&a)?.unwrap().is_nan()); + assert!(a.cross_entropy(&b)?.is_nan()); + assert!(b.cross_entropy(&a)?.is_nan()); + assert!(a.kl_divergence(&b)?.is_nan()); + assert!(b.kl_divergence(&a)?.is_nan()); Ok(()) } @@ -284,20 +288,19 @@ mod tests { } #[test] - fn test_cross_entropy_and_kl_with_empty_array_of_floats() -> Result<(), ShapeMismatch> { + fn test_cross_entropy_and_kl_with_empty_array_of_floats() { let p: Array1 = array![]; let q: Array1 = array![]; - assert!(p.cross_entropy(&q)?.is_none()); - assert!(p.kl_divergence(&q)?.is_none()); - Ok(()) + assert!(p.cross_entropy(&q).unwrap_err().is_empty_input()); + assert!(p.kl_divergence(&q).unwrap_err().is_empty_input()); } #[test] - fn test_cross_entropy_and_kl_with_negative_qs() -> Result<(), ShapeMismatch> { + fn test_cross_entropy_and_kl_with_negative_qs() -> Result<(), MultiInputError> { let p = array![1.]; let q = array![-1.]; - let cross_entropy: f64 = p.cross_entropy(&q)?.unwrap(); - let kl_divergence: f64 = p.kl_divergence(&q)?.unwrap(); + let cross_entropy: f64 = p.cross_entropy(&q)?; + let kl_divergence: f64 = p.kl_divergence(&q)?; assert!(cross_entropy.is_nan()); assert!(kl_divergence.is_nan()); Ok(()) @@ -320,26 +323,26 @@ mod tests { } #[test] - fn test_cross_entropy_and_kl_with_zeroes_p() -> Result<(), ShapeMismatch> { + fn test_cross_entropy_and_kl_with_zeroes_p() -> Result<(), MultiInputError> { let p = array![0., 0.]; let q = array![0., 0.5]; - assert_eq!(p.cross_entropy(&q)?.unwrap(), 0.); - assert_eq!(p.kl_divergence(&q)?.unwrap(), 0.); + assert_eq!(p.cross_entropy(&q)?, 0.); + assert_eq!(p.kl_divergence(&q)?, 0.); Ok(()) } #[test] fn test_cross_entropy_and_kl_with_zeroes_q_and_different_data_ownership( - ) -> Result<(), ShapeMismatch> { + ) -> Result<(), MultiInputError> { let p = array![0.5, 0.5]; let mut q = array![0.5, 0.]; - assert_eq!(p.cross_entropy(&q.view_mut())?.unwrap(), f64::INFINITY); - assert_eq!(p.kl_divergence(&q.view_mut())?.unwrap(), f64::INFINITY); + assert_eq!(p.cross_entropy(&q.view_mut())?, f64::INFINITY); + assert_eq!(p.kl_divergence(&q.view_mut())?, f64::INFINITY); Ok(()) } #[test] - fn test_cross_entropy() -> Result<(), ShapeMismatch> { + fn test_cross_entropy() -> Result<(), MultiInputError> { // Arrays of probability values - normalized and positive. let p: Array1 = array![ 0.05340169, 0.02508511, 0.03460454, 0.00352313, 0.07837615, 0.05859495, 0.05782189, @@ -356,16 +359,12 @@ mod tests { // Computed using scipy.stats.entropy(p) + scipy.stats.entropy(p, q) let expected_cross_entropy = 3.385347705020779; - assert_abs_diff_eq!( - p.cross_entropy(&q)?.unwrap(), - expected_cross_entropy, - epsilon = 1e-6 - ); + assert_abs_diff_eq!(p.cross_entropy(&q)?, expected_cross_entropy, epsilon = 1e-6); Ok(()) } #[test] - fn test_kl() -> Result<(), ShapeMismatch> { + fn test_kl() -> Result<(), MultiInputError> { // Arrays of probability values - normalized and positive. let p: Array1 = array![ 0.00150472, 0.01388706, 0.03495376, 0.03264211, 0.03067355, 0.02183501, 0.00137516, @@ -390,7 +389,7 @@ mod tests { // Computed using scipy.stats.entropy(p, q) let expected_kl = 0.3555862567800096; - assert_abs_diff_eq!(p.kl_divergence(&q)?.unwrap(), expected_kl, epsilon = 1e-6); + assert_abs_diff_eq!(p.kl_divergence(&q)?, expected_kl, epsilon = 1e-6); Ok(()) } } diff --git a/src/errors.rs b/src/errors.rs index 4bbeea46..d89112a5 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -2,10 +2,50 @@ use std::error::Error; use std::fmt; -#[derive(Debug)] +/// An error that indicates that the input array was empty. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct EmptyInput; + +impl fmt::Display for EmptyInput { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "Empty input.") + } +} + +impl Error for EmptyInput {} + +/// An error computing a minimum/maximum value. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum MinMaxError { + /// The input was empty. + EmptyInput, + /// The ordering between a tested pair of values was undefined. + UndefinedOrder, +} + +impl fmt::Display for MinMaxError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + MinMaxError::EmptyInput => write!(f, "Empty input."), + MinMaxError::UndefinedOrder => { + write!(f, "Undefined ordering between a tested pair of values.") + } + } + } +} + +impl Error for MinMaxError {} + +impl From for MinMaxError { + fn from(_: EmptyInput) -> MinMaxError { + MinMaxError::EmptyInput + } +} + /// An error used by methods and functions that take two arrays as argument and /// expect them to have exactly the same shape /// (e.g. `ShapeMismatch` is raised when `a.shape() == b.shape()` evaluates to `False`). +#[derive(Clone, Debug)] pub struct ShapeMismatch { pub first_shape: Vec, pub second_shape: Vec, @@ -22,3 +62,53 @@ impl fmt::Display for ShapeMismatch { } impl Error for ShapeMismatch {} + +/// An error for methods that take multiple non-empty array inputs. +#[derive(Clone, Debug)] +pub enum MultiInputError { + /// One or more of the arrays were empty. + EmptyInput, + /// The arrays did not have the same shape. + ShapeMismatch(ShapeMismatch), +} + +impl MultiInputError { + /// Returns whether `self` is the `EmptyInput` variant. + pub fn is_empty_input(&self) -> bool { + match self { + MultiInputError::EmptyInput => true, + _ => false, + } + } + + /// Returns whether `self` is the `ShapeMismatch` variant. + pub fn is_shape_mismatch(&self) -> bool { + match self { + MultiInputError::ShapeMismatch(_) => true, + _ => false, + } + } +} + +impl fmt::Display for MultiInputError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + MultiInputError::EmptyInput => write!(f, "Empty input."), + MultiInputError::ShapeMismatch(e) => write!(f, "Shape mismatch: {}", e), + } + } +} + +impl Error for MultiInputError {} + +impl From for MultiInputError { + fn from(_: EmptyInput) -> Self { + MultiInputError::EmptyInput + } +} + +impl From for MultiInputError { + fn from(err: ShapeMismatch) -> Self { + MultiInputError::ShapeMismatch(err) + } +} diff --git a/src/histogram/errors.rs b/src/histogram/errors.rs index 08f08ac9..e454e715 100644 --- a/src/histogram/errors.rs +++ b/src/histogram/errors.rs @@ -1,3 +1,4 @@ +use crate::errors::{EmptyInput, MinMaxError}; use std::error; use std::fmt; @@ -17,8 +18,32 @@ impl error::Error for BinNotFound { } } +/// Error computing the set of histogram bins according to a specific strategy. #[derive(Debug, Clone)] -pub struct StrategyError; +pub enum StrategyError { + /// The input array was empty. + EmptyInput, + /// Other error. + Other, +} + +impl StrategyError { + /// Returns whether `self` is the `EmptyInput` variant. + pub fn is_empty_input(&self) -> bool { + match self { + StrategyError::EmptyInput => true, + _ => false, + } + } + + /// Returns whether `self` is the `Other` variant. + pub fn is_other(&self) -> bool { + match self { + StrategyError::Other => true, + _ => false, + } + } +} impl fmt::Display for StrategyError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { @@ -31,3 +56,18 @@ impl error::Error for StrategyError { "The strategy failed to determine a non-zero bin width." } } + +impl From for StrategyError { + fn from(_: EmptyInput) -> Self { + StrategyError::EmptyInput + } +} + +impl From for StrategyError { + fn from(err: MinMaxError) -> StrategyError { + match err { + MinMaxError::EmptyInput => StrategyError::EmptyInput, + MinMaxError::UndefinedOrder => StrategyError::Other, + } + } +} diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index 06914d77..e1b9b66b 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -181,7 +181,7 @@ where let bin_builders: Option> = array .axis_iter(Axis(1)) .map(|data| match B::from_array(&data) { - Ok(Some(builder)) => Some(builder), + Ok(builder) => Some(builder), _ => None, }) .collect(); diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index b2e1361b..3f6358ee 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -41,11 +41,11 @@ pub trait BinsBuildingStrategy { /// Given some observations in a 1-dimensional array it returns a `BinsBuildingStrategy` /// that has learned the required parameter to build a collection of [`Bins`]. /// - /// It returns `None` if it is not possible to build a collection of [`Bins`] given - /// the observed data according to the chosen strategy. + /// It returns `Err` if it is not possible to build a collection of + /// [`Bins`] given the observed data according to the chosen strategy. /// /// [`Bins`]: ../struct.Bins.html - fn from_array(array: &ArrayBase) -> Result, StrategyError> + fn from_array(array: &ArrayBase) -> Result where S: Data, Self: std::marker::Sized; @@ -153,11 +153,11 @@ impl EquiSpaced where T: Ord + Clone + FromPrimitive + NumOps + Zero, { - /// Returns `Err(StrategyError)` if `bin_width<=0` or `min` >= `max`. + /// Returns `Err(StrategyError::Other)` if `bin_width<=0` or `min` >= `max`. /// Returns `Ok(Self)` otherwise. fn new(bin_width: T, min: T, max: T) -> Result { if (bin_width <= T::zero()) | (min >= max) { - Err(StrategyError) + Err(StrategyError::Other) } else { Ok(Self { bin_width, @@ -198,23 +198,20 @@ where { type Elem = T; - /// Returns `Err(StrategyError)` if the array is constant. - /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Err(StrategyError::Other)` if the array is constant. + /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result, StrategyError> + fn from_array(a: &ArrayBase) -> Result where S: Data, { let n_elems = a.len(); let n_bins = (n_elems as f64).sqrt().round() as usize; - match (a.min(), a.max()) { - (Some(min), Some(max)) => { - let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Ok(Some(Self { builder })) - } - _ => Ok(None), - } + let min = a.min()?; + let max = a.max()?; + let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Ok(Self { builder }) } fn build(&self) -> Bins { @@ -242,23 +239,20 @@ where { type Elem = T; - /// Returns `Err(StrategyError)` if the array is constant. - /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Err(StrategyError::Other)` if the array is constant. + /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result, StrategyError> + fn from_array(a: &ArrayBase) -> Result where S: Data, { let n_elems = a.len(); let n_bins = (2. * (n_elems as f64).powf(1. / 3.)).round() as usize; - match (a.min(), a.max()) { - (Some(min), Some(max)) => { - let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Ok(Some(Self { builder })) - } - _ => Ok(None), - } + let min = a.min()?; + let max = a.max()?; + let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Ok(Self { builder }) } fn build(&self) -> Bins { @@ -286,23 +280,20 @@ where { type Elem = T; - /// Returns `Err(StrategyError)` if the array is constant. - /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Err(StrategyError::Other)` if the array is constant. + /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result, StrategyError> + fn from_array(a: &ArrayBase) -> Result where S: Data, { let n_elems = a.len(); let n_bins = (n_elems as f64).log2().round() as usize + 1; - match (a.min(), a.max()) { - (Some(min), Some(max)) => { - let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Ok(Some(Self { builder })) - } - _ => Ok(None), - } + let min = a.min()?; + let max = a.max()?; + let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins); + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Ok(Self { builder }) } fn build(&self) -> Bins { @@ -330,16 +321,16 @@ where { type Elem = T; - /// Returns `Err(StrategyError)` if `IQR==0`. - /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Err(StrategyError::Other)` if `IQR==0`. + /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result, StrategyError> + fn from_array(a: &ArrayBase) -> Result where S: Data, { let n_points = a.len(); if n_points == 0 { - return Ok(None); + return Err(StrategyError::EmptyInput); } let mut a_copy = a.to_owned(); @@ -348,13 +339,10 @@ where let iqr = third_quartile - first_quartile; let bin_width = FreedmanDiaconis::compute_bin_width(n_points, iqr); - match (a.min(), a.max()) { - (Some(min), Some(max)) => { - let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; - Ok(Some(Self { builder })) - } - _ => Ok(None), - } + let min = a.min()?; + let max = a.max()?; + let builder = EquiSpaced::new(bin_width, min.clone(), max.clone())?; + Ok(Self { builder }) } fn build(&self) -> Bins { @@ -388,36 +376,33 @@ where { type Elem = T; - /// Returns `Err(StrategyError)` if `IQR==0`. - /// Returns `Ok(None)` if `a.len()==0`. + /// Returns `Err(StrategyError::Other)` if `IQR==0`. + /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result, StrategyError> + fn from_array(a: &ArrayBase) -> Result where S: Data, { let fd_builder = FreedmanDiaconis::from_array(&a); let sturges_builder = Sturges::from_array(&a); match (fd_builder, sturges_builder) { - (Ok(None), Ok(Some(sturges_builder))) | (Err(_), Ok(Some(sturges_builder))) => { + (Err(_), Ok(sturges_builder)) => { let builder = SturgesOrFD::Sturges(sturges_builder); - Ok(Some(Self { builder })) + Ok(Self { builder }) } - (Ok(Some(fd_builder)), Ok(None)) | (Ok(Some(fd_builder)), Err(_)) => { + (Ok(fd_builder), Err(_)) => { let builder = SturgesOrFD::FreedmanDiaconis(fd_builder); - Ok(Some(Self { builder })) + Ok(Self { builder }) } - (Ok(Some(fd_builder)), Ok(Some(sturges_builder))) => { + (Ok(fd_builder), Ok(sturges_builder)) => { let builder = if fd_builder.bin_width() > sturges_builder.bin_width() { SturgesOrFD::Sturges(sturges_builder) } else { SturgesOrFD::FreedmanDiaconis(fd_builder) }; - Ok(Some(Self { builder })) + Ok(Self { builder }) } - (Err(_), Err(_)) => Err(StrategyError), - (Ok(None), Ok(None)) => Ok(None), - // Unreachable - (Err(_), Ok(None)) | (Ok(None), Err(_)) => Err(StrategyError), + (Err(err), Err(_)) => Err(err), } } @@ -489,13 +474,16 @@ mod sqrt_tests { #[test] fn constant_array_are_bad() { - assert!(Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); + assert!(Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]) + .unwrap_err() + .is_other()); } #[test] - fn empty_arrays_are_bad() -> Result<(), StrategyError> { - assert!(Sqrt::::from_array(&array![])?.is_none()); - Ok(()) + fn empty_arrays_are_bad() { + assert!(Sqrt::::from_array(&array![]) + .unwrap_err() + .is_empty_input()); } } @@ -506,13 +494,16 @@ mod rice_tests { #[test] fn constant_array_are_bad() { - assert!(Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); + assert!(Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]) + .unwrap_err() + .is_other()); } #[test] - fn empty_arrays_are_bad() -> Result<(), StrategyError> { - assert!(Rice::::from_array(&array![])?.is_none()); - Ok(()) + fn empty_arrays_are_bad() { + assert!(Rice::::from_array(&array![]) + .unwrap_err() + .is_empty_input()); } } @@ -523,13 +514,16 @@ mod sturges_tests { #[test] fn constant_array_are_bad() { - assert!(Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); + assert!(Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]) + .unwrap_err() + .is_other()); } #[test] - fn empty_arrays_are_bad() -> Result<(), StrategyError> { - assert!(Sturges::::from_array(&array![])?.is_none()); - Ok(()) + fn empty_arrays_are_bad() { + assert!(Sturges::::from_array(&array![]) + .unwrap_err() + .is_empty_input()); } } @@ -540,21 +534,25 @@ mod fd_tests { #[test] fn constant_array_are_bad() { - assert!(FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); + assert!(FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]) + .unwrap_err() + .is_other()); } #[test] fn zero_iqr_is_bad() { assert!( FreedmanDiaconis::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]) - .is_err() + .unwrap_err() + .is_other() ); } #[test] - fn empty_arrays_are_bad() -> Result<(), StrategyError> { - assert!(FreedmanDiaconis::::from_array(&array![])?.is_none()); - Ok(()) + fn empty_arrays_are_bad() { + assert!(FreedmanDiaconis::::from_array(&array![]) + .unwrap_err() + .is_empty_input()); } } @@ -565,7 +563,9 @@ mod auto_tests { #[test] fn constant_array_are_bad() { - assert!(Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]).is_err()); + assert!(Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]) + .unwrap_err() + .is_other()); } #[test] @@ -574,8 +574,9 @@ mod auto_tests { } #[test] - fn empty_arrays_cause_panic() -> Result<(), StrategyError> { - assert!(Auto::::from_array(&array![])?.is_none()); - Ok(()) + fn empty_arrays_are_bad() { + assert!(Auto::::from_array(&array![]) + .unwrap_err() + .is_empty_input()); } } diff --git a/src/quantile.rs b/src/quantile.rs index 1b4b4fd6..626b27f9 100644 --- a/src/quantile.rs +++ b/src/quantile.rs @@ -1,3 +1,4 @@ +use crate::errors::{EmptyInput, MinMaxError, MinMaxError::UndefinedOrder}; use interpolate::Interpolate; use ndarray::prelude::*; use ndarray::{s, Data, DataMut, RemoveAxis}; @@ -184,11 +185,11 @@ where { /// Finds the index of the minimum value of the array. /// - /// Returns `None` if any of the pairwise orderings tested by the function - /// are undefined. (For example, this occurs if there are any - /// floating-point NaN values in the array.) + /// Returns `Err(MinMaxError::UndefinedOrder)` if any of the pairwise + /// orderings tested by the function are undefined. (For example, this + /// occurs if there are any floating-point NaN values in the array.) /// - /// Returns `None` if the array is empty. + /// Returns `Err(MinMaxError::EmptyInput)` if the array is empty. /// /// Even if there are multiple (equal) elements that are minima, only one /// index is returned. (Which one is returned is unspecified and may depend @@ -205,9 +206,9 @@ where /// /// let a = array![[1., 3., 5.], /// [2., 0., 6.]]; - /// assert_eq!(a.argmin(), Some((1, 1))); + /// assert_eq!(a.argmin(), Ok((1, 1))); /// ``` - fn argmin(&self) -> Option + fn argmin(&self) -> Result where A: PartialOrd; @@ -240,16 +241,16 @@ where /// Finds the elementwise minimum of the array. /// - /// Returns `None` if any of the pairwise orderings tested by the function - /// are undefined. (For example, this occurs if there are any - /// floating-point NaN values in the array.) + /// Returns `Err(MinMaxError::UndefinedOrder)` if any of the pairwise + /// orderings tested by the function are undefined. (For example, this + /// occurs if there are any floating-point NaN values in the array.) /// - /// Additionally, returns `None` if the array is empty. + /// Returns `Err(MinMaxError::EmptyInput)` if the array is empty. /// /// Even if there are multiple (equal) elements that are minima, only one /// is returned. (Which one is returned is unspecified and may depend on /// the memory layout of the array.) - fn min(&self) -> Option<&A> + fn min(&self) -> Result<&A, MinMaxError> where A: PartialOrd; @@ -269,11 +270,11 @@ where /// Finds the index of the maximum value of the array. /// - /// Returns `None` if any of the pairwise orderings tested by the function - /// are undefined. (For example, this occurs if there are any - /// floating-point NaN values in the array.) + /// Returns `Err(MinMaxError::UndefinedOrder)` if any of the pairwise + /// orderings tested by the function are undefined. (For example, this + /// occurs if there are any floating-point NaN values in the array.) /// - /// Returns `None` if the array is empty. + /// Returns `Err(MinMaxError::EmptyInput)` if the array is empty. /// /// Even if there are multiple (equal) elements that are maxima, only one /// index is returned. (Which one is returned is unspecified and may depend @@ -290,9 +291,9 @@ where /// /// let a = array![[1., 3., 7.], /// [2., 5., 6.]]; - /// assert_eq!(a.argmax(), Some((0, 2))); + /// assert_eq!(a.argmax(), Ok((0, 2))); /// ``` - fn argmax(&self) -> Option + fn argmax(&self) -> Result where A: PartialOrd; @@ -325,16 +326,16 @@ where /// Finds the elementwise maximum of the array. /// - /// Returns `None` if any of the pairwise orderings tested by the function - /// are undefined. (For example, this occurs if there are any - /// floating-point NaN values in the array.) + /// Returns `Err(MinMaxError::UndefinedOrder)` if any of the pairwise + /// orderings tested by the function are undefined. (For example, this + /// occurs if there are any floating-point NaN values in the array.) /// - /// Additionally, returns `None` if the array is empty. + /// Returns `Err(EmptyInput)` if the array is empty. /// /// Even if there are multiple (equal) elements that are maxima, only one /// is returned. (Which one is returned is unspecified and may depend on /// the memory layout of the array.) - fn max(&self) -> Option<&A> + fn max(&self) -> Result<&A, MinMaxError> where A: PartialOrd; @@ -406,21 +407,21 @@ where S: Data, D: Dimension, { - fn argmin(&self) -> Option + fn argmin(&self) -> Result where A: PartialOrd, { - let mut current_min = self.first()?; + let mut current_min = self.first().ok_or(EmptyInput)?; let mut current_pattern_min = D::zeros(self.ndim()).into_pattern(); for (pattern, elem) in self.indexed_iter() { - if elem.partial_cmp(current_min)? == cmp::Ordering::Less { + if elem.partial_cmp(current_min).ok_or(UndefinedOrder)? == cmp::Ordering::Less { current_pattern_min = pattern; current_min = elem } } - Some(current_pattern_min) + Ok(current_pattern_min) } fn argmin_skipnan(&self) -> Option @@ -445,14 +446,17 @@ where } } - fn min(&self) -> Option<&A> + fn min(&self) -> Result<&A, MinMaxError> where A: PartialOrd, { - let first = self.first()?; - self.fold(Some(first), |acc, elem| match elem.partial_cmp(acc?)? { - cmp::Ordering::Less => Some(elem), - _ => acc, + let first = self.first().ok_or(EmptyInput)?; + self.fold(Ok(first), |acc, elem| { + let acc = acc?; + match elem.partial_cmp(acc).ok_or(UndefinedOrder)? { + cmp::Ordering::Less => Ok(elem), + _ => Ok(acc), + } }) } @@ -470,21 +474,21 @@ where })) } - fn argmax(&self) -> Option + fn argmax(&self) -> Result where A: PartialOrd, { - let mut current_max = self.first()?; + let mut current_max = self.first().ok_or(EmptyInput)?; let mut current_pattern_max = D::zeros(self.ndim()).into_pattern(); for (pattern, elem) in self.indexed_iter() { - if elem.partial_cmp(current_max)? == cmp::Ordering::Greater { + if elem.partial_cmp(current_max).ok_or(UndefinedOrder)? == cmp::Ordering::Greater { current_pattern_max = pattern; current_max = elem } } - Some(current_pattern_max) + Ok(current_pattern_max) } fn argmax_skipnan(&self) -> Option @@ -509,14 +513,17 @@ where } } - fn max(&self) -> Option<&A> + fn max(&self) -> Result<&A, MinMaxError> where A: PartialOrd, { - let first = self.first()?; - self.fold(Some(first), |acc, elem| match elem.partial_cmp(acc?)? { - cmp::Ordering::Greater => Some(elem), - _ => acc, + let first = self.first().ok_or(EmptyInput)?; + self.fold(Ok(first), |acc, elem| { + let acc = acc?; + match elem.partial_cmp(acc).ok_or(UndefinedOrder)? { + cmp::Ordering::Greater => Ok(elem), + _ => Ok(acc), + } }) } @@ -619,10 +626,10 @@ where /// - worst case: O(`m`^2); /// where `m` is the number of elements in the array. /// - /// Returns `None` if the array is empty. + /// Returns `Err(EmptyInput)` if the array is empty. /// /// **Panics** if `q` is not between `0.` and `1.` (inclusive). - fn quantile_mut(&mut self, q: f64) -> Option + fn quantile_mut(&mut self, q: f64) -> Result where A: Ord + Clone, S: DataMut, @@ -633,16 +640,16 @@ impl Quantile1dExt for ArrayBase where S: Data, { - fn quantile_mut(&mut self, q: f64) -> Option + fn quantile_mut(&mut self, q: f64) -> Result where A: Ord + Clone, S: DataMut, I: Interpolate, { if self.is_empty() { - None + Err(EmptyInput) } else { - Some(self.quantile_axis_mut::(Axis(0), q).into_scalar()) + Ok(self.quantile_axis_mut::(Axis(0), q).into_scalar()) } } } diff --git a/src/summary_statistics/means.rs b/src/summary_statistics/means.rs index f1059efd..a2fed054 100644 --- a/src/summary_statistics/means.rs +++ b/src/summary_statistics/means.rs @@ -1,4 +1,5 @@ use super::SummaryStatisticsExt; +use crate::errors::EmptyInput; use ndarray::{ArrayBase, Data, Dimension}; use num_traits::{Float, FromPrimitive, Zero}; use std::ops::{Add, Div}; @@ -8,28 +9,28 @@ where S: Data, D: Dimension, { - fn mean(&self) -> Option + fn mean(&self) -> Result where A: Clone + FromPrimitive + Add + Div + Zero, { let n_elements = self.len(); if n_elements == 0 { - None + Err(EmptyInput) } else { let n_elements = A::from_usize(n_elements) .expect("Converting number of elements to `A` must not fail."); - Some(self.sum() / n_elements) + Ok(self.sum() / n_elements) } } - fn harmonic_mean(&self) -> Option + fn harmonic_mean(&self) -> Result where A: Float + FromPrimitive, { self.map(|x| x.recip()).mean().map(|x| x.recip()) } - fn geometric_mean(&self) -> Option + fn geometric_mean(&self) -> Result where A: Float + FromPrimitive, { @@ -40,6 +41,7 @@ where #[cfg(test)] mod tests { use super::SummaryStatisticsExt; + use crate::errors::EmptyInput; use approx::abs_diff_eq; use ndarray::{array, Array1}; use noisy_float::types::N64; @@ -56,17 +58,17 @@ mod tests { #[test] fn test_means_with_empty_array_of_floats() { let a: Array1 = array![]; - assert!(a.mean().is_none()); - assert!(a.harmonic_mean().is_none()); - assert!(a.geometric_mean().is_none()); + assert_eq!(a.mean(), Err(EmptyInput)); + assert_eq!(a.harmonic_mean(), Err(EmptyInput)); + assert_eq!(a.geometric_mean(), Err(EmptyInput)); } #[test] fn test_means_with_empty_array_of_noisy_floats() { let a: Array1 = array![]; - assert!(a.mean().is_none()); - assert!(a.harmonic_mean().is_none()); - assert!(a.geometric_mean().is_none()); + assert_eq!(a.mean(), Err(EmptyInput)); + assert_eq!(a.harmonic_mean(), Err(EmptyInput)); + assert_eq!(a.geometric_mean(), Err(EmptyInput)); } #[test] diff --git a/src/summary_statistics/mod.rs b/src/summary_statistics/mod.rs index 6aca865f..30d20b89 100644 --- a/src/summary_statistics/mod.rs +++ b/src/summary_statistics/mod.rs @@ -1,4 +1,5 @@ //! Summary statistics (e.g. mean, variance, etc.). +use crate::errors::EmptyInput; use ndarray::{Data, Dimension}; use num_traits::{Float, FromPrimitive, Zero}; use std::ops::{Add, Div}; @@ -18,12 +19,12 @@ where /// n i=1 /// ``` /// - /// If the array is empty, `None` is returned. + /// If the array is empty, `Err(EmptyInput)` is returned. /// /// **Panics** if `A::from_usize()` fails to convert the number of elements in the array. /// /// [`arithmetic mean`]: https://en.wikipedia.org/wiki/Arithmetic_mean - fn mean(&self) -> Option + fn mean(&self) -> Result where A: Clone + FromPrimitive + Add + Div + Zero; @@ -35,12 +36,12 @@ where /// ⎝i=1 ⎠ /// ``` /// - /// If the array is empty, `None` is returned. + /// If the array is empty, `Err(EmptyInput)` is returned. /// /// **Panics** if `A::from_usize()` fails to convert the number of elements in the array. /// /// [`harmonic mean`]: https://en.wikipedia.org/wiki/Harmonic_mean - fn harmonic_mean(&self) -> Option + fn harmonic_mean(&self) -> Result where A: Float + FromPrimitive; @@ -52,12 +53,12 @@ where /// ⎝i=1 ⎠ /// ``` /// - /// If the array is empty, `None` is returned. + /// If the array is empty, `Err(EmptyInput)` is returned. /// /// **Panics** if `A::from_usize()` fails to convert the number of elements in the array. /// /// [`geometric mean`]: https://en.wikipedia.org/wiki/Geometric_mean - fn geometric_mean(&self) -> Option + fn geometric_mean(&self) -> Result where A: Float + FromPrimitive; } diff --git a/tests/quantile.rs b/tests/quantile.rs index 3e5ba53b..05f1c7a3 100644 --- a/tests/quantile.rs +++ b/tests/quantile.rs @@ -5,6 +5,7 @@ extern crate quickcheck; use ndarray::prelude::*; use ndarray_stats::{ + errors::MinMaxError, interpolate::{Higher, Linear, Lower, Midpoint, Nearest}, Quantile1dExt, QuantileExt, }; @@ -13,22 +14,22 @@ use quickcheck::quickcheck; #[test] fn test_argmin() { let a = array![[1, 5, 3], [2, 0, 6]]; - assert_eq!(a.argmin(), Some((1, 1))); + assert_eq!(a.argmin(), Ok((1, 1))); let a = array![[1., 5., 3.], [2., 0., 6.]]; - assert_eq!(a.argmin(), Some((1, 1))); + assert_eq!(a.argmin(), Ok((1, 1))); let a = array![[1., 5., 3.], [2., ::std::f64::NAN, 6.]]; - assert_eq!(a.argmin(), None); + assert_eq!(a.argmin(), Err(MinMaxError::UndefinedOrder)); let a: Array2 = array![[], []]; - assert_eq!(a.argmin(), None); + assert_eq!(a.argmin(), Err(MinMaxError::EmptyInput)); } quickcheck! { fn argmin_matches_min(data: Vec) -> bool { let a = Array1::from(data); - a.argmin().map(|i| a[i]) == a.min().cloned() + a.argmin().map(|i| &a[i]) == a.min() } } @@ -66,13 +67,13 @@ quickcheck! { #[test] fn test_min() { let a = array![[1, 5, 3], [2, 0, 6]]; - assert_eq!(a.min(), Some(&0)); + assert_eq!(a.min(), Ok(&0)); let a = array![[1., 5., 3.], [2., 0., 6.]]; - assert_eq!(a.min(), Some(&0.)); + assert_eq!(a.min(), Ok(&0.)); let a = array![[1., 5., 3.], [2., ::std::f64::NAN, 6.]]; - assert_eq!(a.min(), None); + assert_eq!(a.min(), Err(MinMaxError::UndefinedOrder)); } #[test] @@ -93,22 +94,22 @@ fn test_min_skipnan_all_nan() { #[test] fn test_argmax() { let a = array![[1, 5, 3], [2, 0, 6]]; - assert_eq!(a.argmax(), Some((1, 2))); + assert_eq!(a.argmax(), Ok((1, 2))); let a = array![[1., 5., 3.], [2., 0., 6.]]; - assert_eq!(a.argmax(), Some((1, 2))); + assert_eq!(a.argmax(), Ok((1, 2))); let a = array![[1., 5., 3.], [2., ::std::f64::NAN, 6.]]; - assert_eq!(a.argmax(), None); + assert_eq!(a.argmax(), Err(MinMaxError::UndefinedOrder)); let a: Array2 = array![[], []]; - assert_eq!(a.argmax(), None); + assert_eq!(a.argmax(), Err(MinMaxError::EmptyInput)); } quickcheck! { fn argmax_matches_max(data: Vec) -> bool { let a = Array1::from(data); - a.argmax().map(|i| a[i]) == a.max().cloned() + a.argmax().map(|i| &a[i]) == a.max() } } @@ -149,13 +150,13 @@ quickcheck! { #[test] fn test_max() { let a = array![[1, 5, 7], [2, 0, 6]]; - assert_eq!(a.max(), Some(&7)); + assert_eq!(a.max(), Ok(&7)); let a = array![[1., 5., 7.], [2., 0., 6.]]; - assert_eq!(a.max(), Some(&7.)); + assert_eq!(a.max(), Ok(&7.)); let a = array![[1., 5., 7.], [2., ::std::f64::NAN, 6.]]; - assert_eq!(a.max(), None); + assert_eq!(a.max(), Err(MinMaxError::UndefinedOrder)); } #[test] From 29f56f31e265163a5c77c289d2e9b0cc49d95bd9 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 21:08:16 -0400 Subject: [PATCH 25/29] Rename StrategyError to BinsBuildError --- src/histogram/errors.rs | 34 +++++++++++------------ src/histogram/strategies.rs | 54 ++++++++++++++++++------------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/histogram/errors.rs b/src/histogram/errors.rs index e454e715..8fc81a13 100644 --- a/src/histogram/errors.rs +++ b/src/histogram/errors.rs @@ -18,56 +18,56 @@ impl error::Error for BinNotFound { } } -/// Error computing the set of histogram bins according to a specific strategy. +/// Error computing the set of histogram bins. #[derive(Debug, Clone)] -pub enum StrategyError { +pub enum BinsBuildError { /// The input array was empty. EmptyInput, - /// Other error. - Other, + /// The strategy for computing appropriate bins failed. + Strategy, } -impl StrategyError { +impl BinsBuildError { /// Returns whether `self` is the `EmptyInput` variant. pub fn is_empty_input(&self) -> bool { match self { - StrategyError::EmptyInput => true, + BinsBuildError::EmptyInput => true, _ => false, } } - /// Returns whether `self` is the `Other` variant. - pub fn is_other(&self) -> bool { + /// Returns whether `self` is the `Strategy` variant. + pub fn is_strategy(&self) -> bool { match self { - StrategyError::Other => true, + BinsBuildError::Strategy => true, _ => false, } } } -impl fmt::Display for StrategyError { +impl fmt::Display for BinsBuildError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "The strategy failed to determine a non-zero bin width.") } } -impl error::Error for StrategyError { +impl error::Error for BinsBuildError { fn description(&self) -> &str { "The strategy failed to determine a non-zero bin width." } } -impl From for StrategyError { +impl From for BinsBuildError { fn from(_: EmptyInput) -> Self { - StrategyError::EmptyInput + BinsBuildError::EmptyInput } } -impl From for StrategyError { - fn from(err: MinMaxError) -> StrategyError { +impl From for BinsBuildError { + fn from(err: MinMaxError) -> BinsBuildError { match err { - MinMaxError::EmptyInput => StrategyError::EmptyInput, - MinMaxError::UndefinedOrder => StrategyError::Other, + MinMaxError::EmptyInput => BinsBuildError::EmptyInput, + MinMaxError::UndefinedOrder => BinsBuildError::Strategy, } } } diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 3f6358ee..2d4e45b4 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -20,7 +20,7 @@ //! [`NumPy`]: https://docs.scipy.org/doc/numpy/reference/generated/numpy.histogram_bin_edges.html#numpy.histogram_bin_edges use super::super::interpolate::Nearest; use super::super::{Quantile1dExt, QuantileExt}; -use super::errors::StrategyError; +use super::errors::BinsBuildError; use super::{Bins, Edges}; use ndarray::prelude::*; use ndarray::Data; @@ -45,7 +45,7 @@ pub trait BinsBuildingStrategy { /// [`Bins`] given the observed data according to the chosen strategy. /// /// [`Bins`]: ../struct.Bins.html - fn from_array(array: &ArrayBase) -> Result + fn from_array(array: &ArrayBase) -> Result where S: Data, Self: std::marker::Sized; @@ -153,11 +153,11 @@ impl EquiSpaced where T: Ord + Clone + FromPrimitive + NumOps + Zero, { - /// Returns `Err(StrategyError::Other)` if `bin_width<=0` or `min` >= `max`. + /// Returns `Err(BinsBuildError::Strategy)` if `bin_width<=0` or `min` >= `max`. /// Returns `Ok(Self)` otherwise. - fn new(bin_width: T, min: T, max: T) -> Result { + fn new(bin_width: T, min: T, max: T) -> Result { if (bin_width <= T::zero()) | (min >= max) { - Err(StrategyError::Other) + Err(BinsBuildError::Strategy) } else { Ok(Self { bin_width, @@ -198,10 +198,10 @@ where { type Elem = T; - /// Returns `Err(StrategyError::Other)` if the array is constant. - /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. + /// Returns `Err(BinsBuildError::Strategy)` if the array is constant. + /// Returns `Err(BinsBuildError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result + fn from_array(a: &ArrayBase) -> Result where S: Data, { @@ -239,10 +239,10 @@ where { type Elem = T; - /// Returns `Err(StrategyError::Other)` if the array is constant. - /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. + /// Returns `Err(BinsBuildError::Strategy)` if the array is constant. + /// Returns `Err(BinsBuildError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result + fn from_array(a: &ArrayBase) -> Result where S: Data, { @@ -280,10 +280,10 @@ where { type Elem = T; - /// Returns `Err(StrategyError::Other)` if the array is constant. - /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. + /// Returns `Err(BinsBuildError::Strategy)` if the array is constant. + /// Returns `Err(BinsBuildError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result + fn from_array(a: &ArrayBase) -> Result where S: Data, { @@ -321,16 +321,16 @@ where { type Elem = T; - /// Returns `Err(StrategyError::Other)` if `IQR==0`. - /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. + /// Returns `Err(BinsBuildError::Strategy)` if `IQR==0`. + /// Returns `Err(BinsBuildError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result + fn from_array(a: &ArrayBase) -> Result where S: Data, { let n_points = a.len(); if n_points == 0 { - return Err(StrategyError::EmptyInput); + return Err(BinsBuildError::EmptyInput); } let mut a_copy = a.to_owned(); @@ -376,10 +376,10 @@ where { type Elem = T; - /// Returns `Err(StrategyError::Other)` if `IQR==0`. - /// Returns `Err(StrategyError::EmptyInput)` if `a.len()==0`. + /// Returns `Err(BinsBuildError::Strategy)` if `IQR==0`. + /// Returns `Err(BinsBuildError::EmptyInput)` if `a.len()==0`. /// Returns `Ok(Self)` otherwise. - fn from_array(a: &ArrayBase) -> Result + fn from_array(a: &ArrayBase) -> Result where S: Data, { @@ -476,7 +476,7 @@ mod sqrt_tests { fn constant_array_are_bad() { assert!(Sqrt::from_array(&array![1, 1, 1, 1, 1, 1, 1]) .unwrap_err() - .is_other()); + .is_strategy()); } #[test] @@ -496,7 +496,7 @@ mod rice_tests { fn constant_array_are_bad() { assert!(Rice::from_array(&array![1, 1, 1, 1, 1, 1, 1]) .unwrap_err() - .is_other()); + .is_strategy()); } #[test] @@ -516,7 +516,7 @@ mod sturges_tests { fn constant_array_are_bad() { assert!(Sturges::from_array(&array![1, 1, 1, 1, 1, 1, 1]) .unwrap_err() - .is_other()); + .is_strategy()); } #[test] @@ -536,7 +536,7 @@ mod fd_tests { fn constant_array_are_bad() { assert!(FreedmanDiaconis::from_array(&array![1, 1, 1, 1, 1, 1, 1]) .unwrap_err() - .is_other()); + .is_strategy()); } #[test] @@ -544,7 +544,7 @@ mod fd_tests { assert!( FreedmanDiaconis::from_array(&array![-20, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 20]) .unwrap_err() - .is_other() + .is_strategy() ); } @@ -565,7 +565,7 @@ mod auto_tests { fn constant_array_are_bad() { assert!(Auto::from_array(&array![1, 1, 1, 1, 1, 1, 1]) .unwrap_err() - .is_other()); + .is_strategy()); } #[test] From bca2dc92c138393ca3f4846a2dd2ecc6855897ac Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 21:13:47 -0400 Subject: [PATCH 26/29] Make GridBuilder::from_array return Result This is nice because it doesn't lose information. (Returning an `Option` combines the two error variants into a single case.) --- src/histogram/grid.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/histogram/grid.rs b/src/histogram/grid.rs index e1b9b66b..91caab03 100644 --- a/src/histogram/grid.rs +++ b/src/histogram/grid.rs @@ -1,4 +1,5 @@ use super::bins::Bins; +use super::errors::BinsBuildError; use super::strategies::BinsBuildingStrategy; use itertools::izip; use ndarray::{ArrayBase, Axis, Data, Ix1, Ix2}; @@ -169,23 +170,20 @@ where /// it returns a `GridBuilder` instance that has learned the required parameter /// to build a [`Grid`] according to the specified [`strategy`]. /// - /// It returns `None` if it is not possible to build a [`Grid`] given + /// It returns `Err` if it is not possible to build a [`Grid`] given /// the observed data according to the chosen [`strategy`]. /// /// [`Grid`]: struct.Grid.html /// [`strategy`]: strategies/index.html - pub fn from_array(array: &ArrayBase) -> Option + pub fn from_array(array: &ArrayBase) -> Result where S: Data, { - let bin_builders: Option> = array + let bin_builders = array .axis_iter(Axis(1)) - .map(|data| match B::from_array(&data) { - Ok(builder) => Some(builder), - _ => None, - }) - .collect(); - bin_builders.map(|b| Self { bin_builders: b }) + .map(|data| B::from_array(&data)) + .collect::, BinsBuildError>>()?; + Ok(Self { bin_builders }) } /// Returns a [`Grid`] instance, built accordingly to the specified [`strategy`] From f41b317ba708a315b36b12c3b143e9e6185eec2b Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 31 Mar 2019 21:16:22 -0400 Subject: [PATCH 27/29] Make BinsBuildError enum non-exhaustive Once the `#[non_exhaustive]` attribute is stable, we should replace the hidden enum variant with that attribute on the enum. --- src/histogram/errors.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/histogram/errors.rs b/src/histogram/errors.rs index 8fc81a13..8deb6218 100644 --- a/src/histogram/errors.rs +++ b/src/histogram/errors.rs @@ -25,6 +25,8 @@ pub enum BinsBuildError { EmptyInput, /// The strategy for computing appropriate bins failed. Strategy, + #[doc(hidden)] + __NonExhaustive, } impl BinsBuildError { From c280c6b477bd673d68ed5a58d31f127104629a9c Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Mon, 1 Apr 2019 22:08:09 +0100 Subject: [PATCH 28/29] Use lazy OR operator. --- src/histogram/strategies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index b2e1361b..956a13b8 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -156,7 +156,7 @@ where /// Returns `Err(StrategyError)` if `bin_width<=0` or `min` >= `max`. /// Returns `Ok(Self)` otherwise. fn new(bin_width: T, min: T, max: T) -> Result { - if (bin_width <= T::zero()) | (min >= max) { + if (bin_width <= T::zero()) || (min >= max) { Err(StrategyError) } else { Ok(Self { From 701842d27f1b338a706a5f3c053a15e6f90a5991 Mon Sep 17 00:00:00 2001 From: LukeMathWalker Date: Mon, 1 Apr 2019 22:08:37 +0100 Subject: [PATCH 29/29] Use lazy OR operator. --- src/histogram/strategies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/histogram/strategies.rs b/src/histogram/strategies.rs index 2d4e45b4..93d75a9b 100644 --- a/src/histogram/strategies.rs +++ b/src/histogram/strategies.rs @@ -156,7 +156,7 @@ where /// Returns `Err(BinsBuildError::Strategy)` if `bin_width<=0` or `min` >= `max`. /// Returns `Ok(Self)` otherwise. fn new(bin_width: T, min: T, max: T) -> Result { - if (bin_width <= T::zero()) | (min >= max) { + if (bin_width <= T::zero()) || (min >= max) { Err(BinsBuildError::Strategy) } else { Ok(Self {