Skip to content

Commit 71f416a

Browse files
committed
Remove min/max for A: Ord
The problem with the old methods was that they panicked when the array was empty, which was very problematic. (See rust-ndarray/ndarray#512 for discussion.) The old `min_partialord` and `max_partialord` have been renamed to `min`/`max`.
1 parent fcbe35a commit 71f416a

File tree

3 files changed

+22
-64
lines changed

3 files changed

+22
-64
lines changed

src/histogram/strategies.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,8 @@ impl<T> BinsBuildingStrategy for Sqrt<T>
188188
{
189189
let n_elems = a.len();
190190
let n_bins = (n_elems as f64).sqrt().round() as usize;
191-
let min = a.min().clone();
192-
let max = a.max().clone();
191+
let min = a.min().unwrap().clone();
192+
let max = a.max().unwrap().clone();
193193
let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins);
194194
let builder = EquiSpaced::new(bin_width, min, max);
195195
Self { builder }
@@ -227,8 +227,8 @@ impl<T> BinsBuildingStrategy for Rice<T>
227227
{
228228
let n_elems = a.len();
229229
let n_bins = (2. * (n_elems as f64).powf(1./3.)).round() as usize;
230-
let min = a.min().clone();
231-
let max = a.max().clone();
230+
let min = a.min().unwrap().clone();
231+
let max = a.max().unwrap().clone();
232232
let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins);
233233
let builder = EquiSpaced::new(bin_width, min, max);
234234
Self { builder }
@@ -266,8 +266,8 @@ impl<T> BinsBuildingStrategy for Sturges<T>
266266
{
267267
let n_elems = a.len();
268268
let n_bins = (n_elems as f64).log2().round() as usize + 1;
269-
let min = a.min().clone();
270-
let max = a.max().clone();
269+
let min = a.min().unwrap().clone();
270+
let max = a.max().unwrap().clone();
271271
let bin_width = compute_bin_width(min.clone(), max.clone(), n_bins);
272272
let builder = EquiSpaced::new(bin_width, min, max);
273273
Self { builder }
@@ -311,8 +311,8 @@ impl<T> BinsBuildingStrategy for FreedmanDiaconis<T>
311311
let iqr = third_quartile - first_quartile;
312312

313313
let bin_width = FreedmanDiaconis::compute_bin_width(n_points, iqr);
314-
let min = a_copy.min().clone();
315-
let max = a_copy.max().clone();
314+
let min = a_copy.min().unwrap().clone();
315+
let max = a_copy.max().unwrap().clone();
316316
let builder = EquiSpaced::new(bin_width, min, max);
317317
Self { builder }
318318
}

src/quantile.rs

+8-44
Original file line numberDiff line numberDiff line change
@@ -175,21 +175,14 @@ where
175175
S: Data<Elem = A>,
176176
D: Dimension,
177177
{
178-
/// Finds the elementwise minimum of the array.
179-
///
180-
/// **Panics** if the array is empty.
181-
fn min(&self) -> &A
182-
where
183-
A: Ord;
184-
185178
/// Finds the elementwise minimum of the array.
186179
///
187180
/// Returns `None` if any of the pairwise orderings tested by the function
188181
/// are undefined. (For example, this occurs if there are any
189182
/// floating-point NaN values in the array.)
190183
///
191184
/// Additionally, returns `None` if the array is empty.
192-
fn min_partialord(&self) -> Option<&A>
185+
fn min(&self) -> Option<&A>
193186
where
194187
A: PartialOrd;
195188

@@ -203,21 +196,14 @@ where
203196
A: MaybeNan,
204197
A::NotNan: Ord;
205198

206-
/// Finds the elementwise maximum of the array.
207-
///
208-
/// **Panics** if the array is empty.
209-
fn max(&self) -> &A
210-
where
211-
A: Ord;
212-
213199
/// Finds the elementwise maximum of the array.
214200
///
215201
/// Returns `None` if any of the pairwise orderings tested by the function
216202
/// are undefined. (For example, this occurs if there are any
217203
/// floating-point NaN values in the array.)
218204
///
219205
/// Additionally, returns `None` if the array is empty.
220-
fn max_partialord(&self) -> Option<&A>
206+
fn max(&self) -> Option<&A>
221207
where
222208
A: PartialOrd;
223209

@@ -285,22 +271,11 @@ where
285271
S: Data<Elem = A>,
286272
D: Dimension,
287273
{
288-
fn min(&self) -> &A
289-
where
290-
A: Ord,
291-
{
292-
let first = self
293-
.iter()
294-
.next()
295-
.expect("Attempted to find min of empty array.");
296-
self.fold(first, |acc, elem| if elem < acc { elem } else { acc })
297-
}
298-
299-
fn min_partialord(&self) -> Option<&A>
274+
fn min(&self) -> Option<&A>
300275
where
301276
A: PartialOrd,
302277
{
303-
let first = self.iter().next()?;
278+
let first = self.first()?;
304279
self.fold(Some(first), |acc, elem| match elem.partial_cmp(acc?)? {
305280
cmp::Ordering::Less => Some(elem),
306281
_ => acc,
@@ -312,7 +287,7 @@ where
312287
A: MaybeNan,
313288
A::NotNan: Ord,
314289
{
315-
let first = self.iter().next().and_then(|v| v.try_as_not_nan());
290+
let first = self.first().and_then(|v| v.try_as_not_nan());
316291
A::from_not_nan_ref_opt(self.fold_skipnan(first, |acc, elem| {
317292
Some(match acc {
318293
Some(acc) => acc.min(elem),
@@ -321,22 +296,11 @@ where
321296
}))
322297
}
323298

324-
fn max(&self) -> &A
325-
where
326-
A: Ord,
327-
{
328-
let first = self
329-
.iter()
330-
.next()
331-
.expect("Attempted to find max of empty array.");
332-
self.fold(first, |acc, elem| if elem > acc { elem } else { acc })
333-
}
334-
335-
fn max_partialord(&self) -> Option<&A>
299+
fn max(&self) -> Option<&A>
336300
where
337301
A: PartialOrd,
338302
{
339-
let first = self.iter().next()?;
303+
let first = self.first()?;
340304
self.fold(Some(first), |acc, elem| match elem.partial_cmp(acc?)? {
341305
cmp::Ordering::Greater => Some(elem),
342306
_ => acc,
@@ -348,7 +312,7 @@ where
348312
A: MaybeNan,
349313
A::NotNan: Ord,
350314
{
351-
let first = self.iter().next().and_then(|v| v.try_as_not_nan());
315+
let first = self.first().and_then(|v| v.try_as_not_nan());
352316
A::from_not_nan_ref_opt(self.fold_skipnan(first, |acc, elem| {
353317
Some(match acc {
354318
Some(acc) => acc.max(elem),

tests/quantile.rs

+6-12
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,13 @@ use ndarray_stats::{
1111
#[test]
1212
fn test_min() {
1313
let a = array![[1, 5, 3], [2, 0, 6]];
14-
assert_eq!(a.min(), &0);
15-
}
14+
assert_eq!(a.min(), Some(&0));
1615

17-
#[test]
18-
fn test_min_partialord() {
1916
let a = array![[1., 5., 3.], [2., 0., 6.]];
20-
assert_eq!(a.min_partialord(), Some(&0.));
17+
assert_eq!(a.min(), Some(&0.));
2118

2219
let a = array![[1., 5., 3.], [2., ::std::f64::NAN, 6.]];
23-
assert_eq!(a.min_partialord(), None);
20+
assert_eq!(a.min(), None);
2421
}
2522

2623
#[test]
@@ -41,16 +38,13 @@ fn test_min_skipnan_all_nan() {
4138
#[test]
4239
fn test_max() {
4340
let a = array![[1, 5, 7], [2, 0, 6]];
44-
assert_eq!(a.max(), &7);
45-
}
41+
assert_eq!(a.max(), Some(&7));
4642

47-
#[test]
48-
fn test_max_partialord() {
4943
let a = array![[1., 5., 7.], [2., 0., 6.]];
50-
assert_eq!(a.max_partialord(), Some(&7.));
44+
assert_eq!(a.max(), Some(&7.));
5145

5246
let a = array![[1., 5., 7.], [2., ::std::f64::NAN, 6.]];
53-
assert_eq!(a.max_partialord(), None);
47+
assert_eq!(a.max(), None);
5448
}
5549

5650
#[test]

0 commit comments

Comments
 (0)