Merge pull request #630 from jturner314/improve-space-constructors

Improve linspace, range, logspace, geomspace

Author: jturner314

src/geomspace.rs

@@ -66,41 +66,40 @@ impl<F> ExactSizeIterator for Geomspace<F> where Geomspace<F>: Iterator {}
 
 /// An iterator of a sequence of geometrically spaced values.
 ///
-/// The `Geomspace` has `n` elements, where the first element is `a` and the
-/// last element is `b`.
+/// The `Geomspace` has `n` geometrically spaced elements from `start` to `end`
+/// (inclusive).
 ///
-/// Iterator element type is `F`, where `F` must be either `f32` or `f64`.
+/// The iterator element type is `F`, where `F` must implement `Float`, e.g.
+/// `f32` or `f64`.
 ///
-/// **Panics** if the interval `[a, b]` contains zero (including the end points).
+/// Returns `None` if `start` and `end` have different signs or if either one
+/// is zero. Conceptually, this means that in order to obtain a `Some` result,
+/// `end / start` must be positive.
+///
+/// **Panics** if converting `n - 1` to type `F` fails.
 #[inline]
-pub fn geomspace<F>(a: F, b: F, n: usize) -> Geomspace<F>
+pub fn geomspace<F>(a: F, b: F, n: usize) -> Option<Geomspace<F>>
 where
     F: Float,
 {
-    assert!(
-        a != F::zero() && b != F::zero(),
-        "Start and/or end of geomspace cannot be zero.",
-    );
-    assert!(
-        a.is_sign_negative() == b.is_sign_negative(),
-        "Logarithmic interval cannot cross 0."
-    );
-
+    if a == F::zero() || b == F::zero() || a.is_sign_negative() != b.is_sign_negative() {
+        return None;
+    }
     let log_a = a.abs().ln();
     let log_b = b.abs().ln();
     let step = if n > 1 {
-        let nf: F = F::from(n).unwrap();
-        (log_b - log_a) / (nf - F::one())
+        let num_steps = F::from(n - 1).expect("Converting number of steps to `A` must not fail.");
+        (log_b - log_a) / num_steps
     } else {
         F::zero()
     };
-    Geomspace {
+    Some(Geomspace {
         sign: a.signum(),
         start: log_a,
         step: step,
         index: 0,
         len: n,
-    }
+    })
 }
 
 #[cfg(test)]
@@ -113,22 +112,22 @@ mod tests {
         use approx::assert_abs_diff_eq;
         use crate::{arr1, Array1};
 
-        let array: Array1<_> = geomspace(1e0, 1e3, 4).collect();
+        let array: Array1<_> = geomspace(1e0, 1e3, 4).unwrap().collect();
         assert_abs_diff_eq!(array, arr1(&[1e0, 1e1, 1e2, 1e3]), epsilon = 1e-12);
 
-        let array: Array1<_> = geomspace(1e3, 1e0, 4).collect();
+        let array: Array1<_> = geomspace(1e3, 1e0, 4).unwrap().collect();
         assert_abs_diff_eq!(array, arr1(&[1e3, 1e2, 1e1, 1e0]), epsilon = 1e-12);
 
-        let array: Array1<_> = geomspace(-1e3, -1e0, 4).collect();
+        let array: Array1<_> = geomspace(-1e3, -1e0, 4).unwrap().collect();
         assert_abs_diff_eq!(array, arr1(&[-1e3, -1e2, -1e1, -1e0]), epsilon = 1e-12);
 
-        let array: Array1<_> = geomspace(-1e0, -1e3, 4).collect();
+        let array: Array1<_> = geomspace(-1e0, -1e3, 4).unwrap().collect();
         assert_abs_diff_eq!(array, arr1(&[-1e0, -1e1, -1e2, -1e3]), epsilon = 1e-12);
     }
 
     #[test]
     fn iter_forward() {
-        let mut iter = geomspace(1.0f64, 1e3, 4);
+        let mut iter = geomspace(1.0f64, 1e3, 4).unwrap();
 
         assert!(iter.size_hint() == (4, Some(4)));
 
@@ -143,7 +142,7 @@ mod tests {
 
     #[test]
     fn iter_backward() {
-        let mut iter = geomspace(1.0f64, 1e3, 4);
+        let mut iter = geomspace(1.0f64, 1e3, 4).unwrap();
 
         assert!(iter.size_hint() == (4, Some(4)));
 
@@ -157,20 +156,17 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
     fn zero_lower() {
-        geomspace(0.0, 1.0, 4);
+        assert!(geomspace(0.0, 1.0, 4).is_none());
     }
 
     #[test]
-    #[should_panic]
     fn zero_upper() {
-        geomspace(1.0, 0.0, 4);
+        assert!(geomspace(1.0, 0.0, 4).is_none());
     }
 
     #[test]
-    #[should_panic]
     fn zero_included() {
-        geomspace(-1.0, 1.0, 4);
+        assert!(geomspace(-1.0, 1.0, 4).is_none());
     }
 }

src/impl_constructors.rs

@@ -67,10 +67,16 @@ impl<S, A> ArrayBase<S, Ix1>
         Self::from_vec(iterable.into_iter().collect())
     }
 
-    /// Create a one-dimensional array from the inclusive interval
-    /// `[start, end]` with `n` elements. `A` must be a floating point type.
+    /// Create a one-dimensional array with `n` evenly spaced elements from
+    /// `start` to `end` (inclusive). `A` must be a floating point type.
     ///
-    /// **Panics** if `n` is greater than `isize::MAX`.
+    /// Note that if `start > end`, the first element will still be `start`,
+    /// and the following elements will be decreasing. This is different from
+    /// the behavior of `std::ops::RangeInclusive`, which interprets `start >
+    /// end` to mean that the range is empty.
+    ///
+    /// **Panics** if `n` is greater than `isize::MAX` or if converting `n - 1`
+    /// to type `A` fails.
     ///
     /// ```rust
     /// use ndarray::{Array, arr1};
@@ -84,9 +90,8 @@ impl<S, A> ArrayBase<S, Ix1>
         Self::from_vec(to_vec(linspace::linspace(start, end, n)))
     }
 
-    /// Create a one-dimensional array from the half-open interval
-    /// `[start, end)` with elements spaced by `step`. `A` must be a floating
-    /// point type.
+    /// Create a one-dimensional array with elements from `start` to `end`
+    /// (exclusive), incrementing by `step`. `A` must be a floating point type.
     ///
     /// **Panics** if the length is greater than `isize::MAX`.
     ///
@@ -102,13 +107,14 @@ impl<S, A> ArrayBase<S, Ix1>
         Self::from_vec(to_vec(linspace::range(start, end, step)))
     }
 
-    /// Create a one-dimensional array with `n` elements logarithmically spaced,
-    /// with the starting value being `base.powf(start)` and the final one being
-    /// `base.powf(end)`. `A` must be a floating point type.
+    /// Create a one-dimensional array with `n` logarithmically spaced
+    /// elements, with the starting value being `base.powf(start)` and the
+    /// final one being `base.powf(end)`. `A` must be a floating point type.
     ///
     /// If `base` is negative, all values will be negative.
     ///
-    /// **Panics** if the length is greater than `isize::MAX`.
+    /// **Panics** if `n` is greater than `isize::MAX` or if converting `n - 1`
+    /// to type `A` fails.
     ///
     /// ```rust
     /// use approx::assert_abs_diff_eq;
@@ -129,32 +135,38 @@ impl<S, A> ArrayBase<S, Ix1>
         Self::from_vec(to_vec(logspace::logspace(base, start, end, n)))
     }
 
-    /// Create a one-dimensional array from the inclusive interval `[start,
-    /// end]` with `n` elements geometrically spaced. `A` must be a floating
-    /// point type.
+    /// Create a one-dimensional array with `n` geometrically spaced elements
+    /// from `start` to `end` (inclusive). `A` must be a floating point type.
     ///
-    /// The interval can be either all positive or all negative; however, it
-    /// cannot contain 0 (including the end points).
+    /// Returns `None` if `start` and `end` have different signs or if either
+    /// one is zero. Conceptually, this means that in order to obtain a `Some`
+    /// result, `end / start` must be positive.
     ///
-    /// **Panics** if `n` is greater than `isize::MAX`.
+    /// **Panics** if `n` is greater than `isize::MAX` or if converting `n - 1`
+    /// to type `A` fails.
     ///
     /// ```rust
     /// use approx::assert_abs_diff_eq;
     /// use ndarray::{Array, arr1};
     ///
+    /// # fn example() -> Option<()> {
     /// # #[cfg(feature = "approx")] {
-    /// let array = Array::geomspace(1e0, 1e3, 4);
+    /// let array = Array::geomspace(1e0, 1e3, 4)?;
     /// assert_abs_diff_eq!(array, arr1(&[1e0, 1e1, 1e2, 1e3]), epsilon = 1e-12);
     ///
-    /// let array = Array::geomspace(-1e3, -1e0, 4);
+    /// let array = Array::geomspace(-1e3, -1e0, 4)?;
     /// assert_abs_diff_eq!(array, arr1(&[-1e3, -1e2, -1e1, -1e0]), epsilon = 1e-12);
     /// # }
+    /// # Some(())
+    /// # }
+    /// #
+    /// # fn main() { example().unwrap() }
     /// ```
-    pub fn geomspace(start: A, end: A, n: usize) -> Self
+    pub fn geomspace(start: A, end: A, n: usize) -> Option<Self>
     where
         A: Float,
     {
-        Self::from_vec(to_vec(geomspace::geomspace(start, end, n)))
+        Some(Self::from_vec(to_vec(geomspace::geomspace(start, end, n)?)))
     }
 }
 

src/linspace.rs

@@ -63,18 +63,19 @@ impl<F> ExactSizeIterator for Linspace<F>
 
 /// Return an iterator of evenly spaced floats.
 ///
-/// The `Linspace` has `n` elements, where the first
-/// element is `a` and the last element is `b`.
+/// The `Linspace` has `n` elements from `a` to `b` (inclusive).
 ///
-/// Iterator element type is `F`, where `F` must be
-/// either `f32` or `f64`.
+/// The iterator element type is `F`, where `F` must implement `Float`, e.g.
+/// `f32` or `f64`.
+///
+/// **Panics** if converting `n - 1` to type `F` fails.
 #[inline]
 pub fn linspace<F>(a: F, b: F, n: usize) -> Linspace<F>
     where F: Float
 {
     let step = if n > 1 {
-        let nf: F = F::from(n).unwrap();
-        (b - a) / (nf - F::one())
+        let num_steps = F::from(n - 1).expect("Converting number of steps to `A` must not fail.");
+        (b - a) / num_steps
     } else {
         F::zero()
     };
@@ -86,13 +87,15 @@ pub fn linspace<F>(a: F, b: F, n: usize) -> Linspace<F>
     }
 }
 
-/// Return an iterator of floats spaced by `step`, from
-/// the half-open interval [a, b).
-/// Numerical reasons can result in `b` being included
-/// in the result.
+/// Return an iterator of floats from `start` to `end` (exclusive),
+/// incrementing by `step`.
+///
+/// Numerical reasons can result in `b` being included in the result.
+///
+/// The iterator element type is `F`, where `F` must implement `Float`, e.g.
+/// `f32` or `f64`.
 ///
-/// Iterator element type is `F`, where `F` must be
-/// either `f32` or `f64`.
+/// **Panics** if converting `((b - a) / step).ceil()` to type `F` fails.
 #[inline]
 pub fn range<F>(a: F, b: F, step: F) -> Linspace<F>
     where F: Float
@@ -102,7 +105,11 @@ pub fn range<F>(a: F, b: F, step: F) -> Linspace<F>
     Linspace {
         start: a,
         step: step,
-        len: steps.to_usize().unwrap(),
+        len: steps.to_usize().expect(
+            "Converting the length to `usize` must not fail. The most likely \
+             cause of this failure is if the sign of `end - start` is \
+             different from the sign of `step`.",
+        ),
         index: 0,
     }
 }

src/logspace.rs

@@ -65,21 +65,24 @@ where
 
 impl<F> ExactSizeIterator for Logspace<F> where Logspace<F>: Iterator {}
 
-/// An iterator of a sequence of logarithmically spaced number.
+/// An iterator of a sequence of logarithmically spaced numbers.
 ///
 /// The `Logspace` has `n` elements, where the first element is `base.powf(a)`
 /// and the last element is `base.powf(b)`.  If `base` is negative, this
 /// iterator will return all negative values.
 ///
-/// Iterator element type is `F`, where `F` must be either `f32` or `f64`.
+/// The iterator element type is `F`, where `F` must implement `Float`, e.g.
+/// `f32` or `f64`.
+///
+/// **Panics** if converting `n - 1` to type `F` fails.
 #[inline]
 pub fn logspace<F>(base: F, a: F, b: F, n: usize) -> Logspace<F>
 where
     F: Float,
 {
     let step = if n > 1 {
-        let nf: F = F::from(n).unwrap();
-        (b - a) / (nf - F::one())
+        let num_steps = F::from(n - 1).expect("Converting number of steps to `A` must not fail.");
+        (b - a) / num_steps
     } else {
         F::zero()
     };