Merge branch 'master' into dual-pivot

Author: n3vu0r

.github/workflows/ci.yml

@@ -0,0 +1,102 @@
+name: Continuous integration
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+env:
+  CARGO_TERM_COLOR: always
+  RUSTFLAGS: "-D warnings"
+
+jobs:
+
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        rust:
+          - stable
+          - beta
+          - nightly
+          - 1.49.0  # MSRV
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ matrix.rust }}
+          override: true
+      - name: Build
+        run: cargo build --verbose
+      - name: Run tests
+        run: cargo test --verbose
+
+  cross_test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        include:
+          # 64-bit, big-endian
+          - rust: stable
+            target: mips64-unknown-linux-gnuabi64
+          # 32-bit, big-endian
+          - rust: stable
+            target: mips-unknown-linux-gnu
+          # 32-bit, little-endian
+          - rust: stable
+            target: i686-unknown-linux-gnu
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ matrix.rust }}
+          target: ${{ matrix.target }}
+          override: true
+      - name: Install cross
+        run: cargo install cross -f
+      - name: Build
+        run: cross build --verbose --target=${{ matrix.target }}
+      - name: Run tests
+        run: cross test --verbose --target=${{ matrix.target }}
+
+  format:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        rust:
+          - stable
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ matrix.rust }}
+          override: true
+          components: rustfmt
+      - name: Rustfmt
+        run: cargo fmt -- --check
+
+  coverage:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        rust:
+          - nightly
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: ${{ matrix.rust }}
+          override: true
+      - name: Install tarpaulin
+        run: cargo install cargo-tarpaulin -f
+      - name: Generate code coverage
+        run: cargo tarpaulin --verbose --all-features --workspace --timeout 120 --out Xml
+      - name: Upload to codecov.io
+        uses: codecov/codecov-action@v1
+        with:
+          fail_ci_if_error: true

.travis.yml

@@ -1,6 +1,6 @@
 [package]
 name = "ndarray-stats"
-version = "0.5.0"
+version = "0.5.1"
 authors = ["Jim Turner <[email protected]>", "LukeMathWalker <[email protected]>"]
 edition = "2018"
 

Cargo.toml

@@ -1,6 +1,5 @@
 # ndarray-stats
 
-[![Build status](https://travis-ci.org/rust-ndarray/ndarray-stats.svg?branch=master)](https://travis-ci.org/rust-ndarray/ndarray-stats)
 [![Coverage](https://codecov.io/gh/rust-ndarray/ndarray-stats/branch/master/graph/badge.svg)](https://codecov.io/gh/rust-ndarray/ndarray-stats)
 [![Dependencies status](https://deps.rs/repo/github/rust-ndarray/ndarray-stats/status.svg)](https://deps.rs/repo/github/rust-ndarray/ndarray-stats)
 [![Crate](https://img.shields.io/crates/v/ndarray-stats.svg)](https://crates.io/crates/ndarray-stats)
@@ -28,11 +27,21 @@ Please feel free to contribute new functionality! A roadmap can be found [here](
 ```toml
 [dependencies]
 ndarray = "0.15"
-ndarray-stats = "0.5"
+ndarray-stats = "0.5.1"
 ```
 
 ## Releases
 
+* **0.5.1**
+  * Fixed bug in implementation of `MaybeNaN::remove_nan_mut` for `f32` and
+    `f64` for views with non-standard layouts. Before this fix, the bug could
+    cause incorrect results, buffer overflows, etc., in this method and others
+    which use it. Thanks to [@JacekCzupyt](https://github.com/JacekCzupyt) for
+    reporting the issue (#89).
+  * Minor docs improvements.
+
+  *Contributors*: [@jturner314](https://github.com/jturner314), [@BenMoon](https://github.com/BenMoon)
+
 * **0.5.0**
   * Breaking changes
     * Minimum supported Rust version: `1.49.0`
@@ -109,7 +118,7 @@ Please feel free to create issues and submit PRs.
 
 ## License
 
-Copyright 2018 `ndarray-stats` developers
+Copyright 2018–2022 `ndarray-stats` developers
 
 Licensed under the [Apache License, Version 2.0](LICENSE-APACHE), or the [MIT
 license](LICENSE-MIT), at your option. You may not use this project except in

README.md

@@ -1,6 +1,7 @@
 use ndarray::prelude::*;
 use ndarray::{s, Data, DataMut, RemoveAxis};
 use noisy_float::types::{N32, N64};
+use std::mem;
 
 /// A number type that can have not-a-number values.
 pub trait MaybeNan: Sized {
@@ -69,6 +70,42 @@ fn remove_nan_mut<A: MaybeNan>(mut view: ArrayViewMut1<'_, A>) -> ArrayViewMut1<
     }
 }
 
+/// Casts a view from one element type to another.
+///
+/// # Panics
+///
+/// Panics if `T` and `U` differ in size or alignment.
+///
+/// # Safety
+///
+/// The caller must ensure that qll elements in `view` are valid values for type `U`.
+unsafe fn cast_view_mut<T, U>(mut view: ArrayViewMut1<'_, T>) -> ArrayViewMut1<'_, U> {
+    assert_eq!(mem::size_of::<T>(), mem::size_of::<U>());
+    assert_eq!(mem::align_of::<T>(), mem::align_of::<U>());
+    let ptr: *mut U = view.as_mut_ptr().cast();
+    let len: usize = view.len_of(Axis(0));
+    let stride: isize = view.stride_of(Axis(0));
+    if len <= 1 {
+        // We can use a stride of `0` because the stride is irrelevant for the `len == 1` case.
+        let stride = 0;
+        ArrayViewMut1::from_shape_ptr([len].strides([stride]), ptr)
+    } else if stride >= 0 {
+        let stride = stride as usize;
+        ArrayViewMut1::from_shape_ptr([len].strides([stride]), ptr)
+    } else {
+        // At this point, stride < 0. We have to construct the view by using the inverse of the
+        // stride and then inverting the axis, since `ArrayViewMut::from_shape_ptr` requires the
+        // stride to be nonnegative.
+        let neg_stride = stride.checked_neg().unwrap() as usize;
+        // This is safe because `ndarray` guarantees that it's safe to offset the
+        // pointer anywhere in the array.
+        let neg_ptr = ptr.offset((len - 1) as isize * stride);
+        let mut v = ArrayViewMut1::from_shape_ptr([len].strides([neg_stride]), neg_ptr);
+        v.invert_axis(Axis(0));
+        v
+    }
+}
+
 macro_rules! impl_maybenan_for_fxx {
     ($fxx:ident, $Nxx:ident) => {
         impl MaybeNan for $fxx {
@@ -102,11 +139,9 @@ macro_rules! impl_maybenan_for_fxx {
 
             fn remove_nan_mut(view: ArrayViewMut1<'_, $fxx>) -> ArrayViewMut1<'_, $Nxx> {
                 let not_nan = remove_nan_mut(view);
-                // This is safe because `remove_nan_mut` has removed the NaN
-                // values, and `$Nxx` is a thin wrapper around `$fxx`.
-                unsafe {
-                    ArrayViewMut1::from_shape_ptr(not_nan.dim(), not_nan.as_ptr() as *mut $Nxx)
-                }
+                // This is safe because `remove_nan_mut` has removed the NaN values, and `$Nxx` is
+                // a thin wrapper around `$fxx`.
+                unsafe { cast_view_mut(not_nan) }
             }
         }
     };

src/maybe_nan/mod.rs

@@ -0,0 +1,31 @@
+use ndarray::prelude::*;
+use ndarray_stats::MaybeNan;
+use noisy_float::types::{n64, N64};
+
+#[test]
+fn remove_nan_mut_nonstandard_layout() {
+    fn eq_unordered(mut a: Vec<N64>, mut b: Vec<N64>) -> bool {
+        a.sort();
+        b.sort();
+        a == b
+    }
+    let a = aview1(&[1., 2., f64::NAN, f64::NAN, 3., f64::NAN, 4., 5.]);
+    {
+        let mut a = a.to_owned();
+        let v = f64::remove_nan_mut(a.slice_mut(s![..;2]));
+        assert!(eq_unordered(v.to_vec(), vec![n64(1.), n64(3.), n64(4.)]));
+    }
+    {
+        let mut a = a.to_owned();
+        let v = f64::remove_nan_mut(a.slice_mut(s![..;-1]));
+        assert!(eq_unordered(
+            v.to_vec(),
+            vec![n64(5.), n64(4.), n64(3.), n64(2.), n64(1.)],
+        ));
+    }
+    {
+        let mut a = a.to_owned();
+        let v = f64::remove_nan_mut(a.slice_mut(s![..;-2]));
+        assert!(eq_unordered(v.to_vec(), vec![n64(5.), n64(2.)]));
+    }
+}