Merge pull request #80 from kngwyu/refactor

kngwyu · web-flow · commit eee5e5a3e9ac · 2018-11-13T16:45:17.000+09:00
Refactor error types
diff --git a/src/array.rs b/src/array.rs
@@ -2,7 +2,7 @@
 use ndarray::*;
 use npyffi::{self, npy_intp, NPY_ORDER, PY_ARRAY_API};
 use num_traits::AsPrimitive;
-use pyo3::{exceptions::TypeError, ffi, prelude::*, types::PyObjectRef};
+use pyo3::{ffi, prelude::*, types::PyObjectRef};
 use pyo3::{PyDowncastError, PyObjectWithToken, ToPyPointer};
 use std::iter::ExactSizeIterator;
 use std::marker::PhantomData;
@@ -116,30 +116,21 @@ impl<'a, T, D> ::std::convert::From<&'a PyArray<T, D>> for &'a PyObjectRef {
 }
 
 impl<'a, T: TypeNum, D: Dimension> FromPyObject<'a> for &'a PyArray<T, D> {
-    // here we do type-check twice
+    // here we do type-check three times
     // 1. Checks if the object is PyArray
     // 2. Checks if the data type of the array is T
+    // 3. Checks if the dimension is same as D
     fn extract(ob: &'a PyObjectRef) -> PyResult<Self> {
         let array = unsafe {
             if npyffi::PyArray_Check(ob.as_ptr()) == 0 {
                 return Err(PyDowncastError.into());
             }
-            if let Some(ndim) = D::NDIM {
-                let ptr = ob.as_ptr() as *mut npyffi::PyArrayObject;
-                if (*ptr).nd as usize != ndim {
-                    return Err(PyErr::new::<TypeError, _>(format!(
-                        "specified dim was {}, but actual dim was {}",
-                        ndim,
-                        (*ptr).nd
-                    )));
-                }
-            }
             &*(ob as *const PyObjectRef as *const PyArray<T, D>)
         };
         array
             .type_check()
             .map(|_| array)
-            .into_pyresult_with(|| "FromPyObject::extract typecheck failed")
+            .into_pyresult_with(|| "[FromPyObject::extract] typecheck failed")
     }
 }
 
@@ -398,6 +389,27 @@ impl<T: TypeNum, D: Dimension> PyArray<T, D> {
         }
     }
 
+    /// Get the immutable view of the internal data of `PyArray`, as slice.
+    /// # Example
+    /// ```
+    /// # extern crate pyo3; extern crate numpy; fn main() {
+    /// use numpy::PyArray;
+    /// let gil = pyo3::Python::acquire_gil();
+    /// let py_array = PyArray::arange(gil.python(), 0, 4, 1).reshape([2, 2]).unwrap();
+    /// assert_eq!(py_array.as_slice(), &[0, 1, 2, 3]);
+    /// # }
+    /// ```
+    pub fn as_slice(&self) -> &[T] {
+        self.type_check_assert();
+        unsafe { ::std::slice::from_raw_parts(self.data(), self.len()) }
+    }
+
+    /// Get the mmutable view of the internal data of `PyArray`, as slice.
+    pub fn as_slice_mut(&self) -> &mut [T] {
+        self.type_check_assert();
+        unsafe { ::std::slice::from_raw_parts_mut(self.data(), self.len()) }
+    }
+
     /// Construct PyArray from `ndarray::ArrayBase`.
     ///
     /// This method allocates memory in Python's heap via numpy api, and then copies all elements
@@ -584,6 +596,22 @@ impl<T: TypeNum, D: Dimension> PyArray<T, D> {
         let python = self.py();
         unsafe { PyArray::from_borrowed_ptr(python, self.as_ptr()) }
     }
+
+    fn type_check_assert(&self) {
+        let type_check = self.type_check();
+        assert!(type_check.is_ok(), "{:?}", type_check);
+    }
+
+    fn type_check(&self) -> Result<(), ErrorKind> {
+        let truth = self.typenum();
+        let dim = self.shape().len();
+        let dim_ok = D::NDIM.map(|n| n == dim).unwrap_or(true);
+        if T::is_same_type(truth) && dim_ok {
+            Ok(())
+        } else {
+            Err(ErrorKind::to_rust(truth, dim, T::npy_data_type(), D::NDIM))
+        }
+    }
 }
 
 impl<T: TypeNum> PyArray<T, Ix1> {
@@ -828,41 +856,6 @@ impl<T: TypeNum, D> PyArray<T, D> {
         NpyDataType::from_i32(self.typenum())
     }
 
-    fn type_check_assert(&self) {
-        let type_check = self.type_check();
-        assert!(type_check.is_ok(), "{:?}", type_check);
-    }
-
-    fn type_check(&self) -> Result<(), ErrorKind> {
-        let truth = self.typenum();
-        if T::is_same_type(truth) {
-            Ok(())
-        } else {
-            Err(ErrorKind::to_rust(truth, T::npy_data_type()))
-        }
-    }
-
-    /// Get the immutable view of the internal data of `PyArray`, as slice.
-    /// # Example
-    /// ```
-    /// # extern crate pyo3; extern crate numpy; fn main() {
-    /// use numpy::PyArray;
-    /// let gil = pyo3::Python::acquire_gil();
-    /// let py_array = PyArray::arange(gil.python(), 0, 4, 1).reshape([2, 2]).unwrap();
-    /// assert_eq!(py_array.as_slice(), &[0, 1, 2, 3]);
-    /// # }
-    /// ```
-    pub fn as_slice(&self) -> &[T] {
-        self.type_check_assert();
-        unsafe { ::std::slice::from_raw_parts(self.data(), self.len()) }
-    }
-
-    /// Get the mmutable view of the internal data of `PyArray`, as slice.
-    pub fn as_slice_mut(&self) -> &mut [T] {
-        self.type_check_assert();
-        unsafe { ::std::slice::from_raw_parts_mut(self.data(), self.len()) }
-    }
-
     /// Copies self into `other`, performing a data-type conversion if necessary.
     /// # Example
     /// ```
diff --git a/src/error.rs b/src/error.rs
@@ -31,74 +31,101 @@ impl<T, E: IntoPyErr> IntoPyResult for Result<T, E> {
     }
 }
 
-/// Represents a shape and format of numpy array.
+/// Represents a shape and dtype of numpy array.
 ///
 /// Only for error formatting.
 #[derive(Debug)]
-pub struct ArrayFormat {
+pub struct ArrayShape {
     pub dims: Box<[usize]>,
     pub dtype: NpyDataType,
 }
 
-impl fmt::Display for ArrayFormat {
+impl ArrayShape {
+    fn boxed_dims(dims: &[usize]) -> Box<[usize]> {
+        dims.into_iter()
+            .map(|&x| x)
+            .collect::<Vec<_>>()
+            .into_boxed_slice()
+    }
+    fn from_array<T: TypeNum, D>(array: &PyArray<T, D>) -> Self {
+        ArrayShape {
+            dims: Self::boxed_dims(array.shape()),
+            dtype: T::npy_data_type(),
+        }
+    }
+    fn from_dims<T: TypeNum, D: ToNpyDims>(dims: D) -> Self {
+        ArrayShape {
+            dims: Self::boxed_dims(dims.slice()),
+            dtype: T::npy_data_type(),
+        }
+    }
+}
+
+impl fmt::Display for ArrayShape {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "dims={:?}, dtype={:?}", self.dims, self.dtype)
     }
 }
 
+/// Represents a dimension and dtype of numpy array.
+///
+/// Only for error formatting.
+#[derive(Debug)]
+pub struct ArrayDim {
+    pub dim: Option<usize>,
+    pub dtype: NpyDataType,
+}
+
+impl fmt::Display for ArrayDim {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if let Some(d) = self.dim {
+            write!(f, "dim={:?}, dtype={:?}", d, self.dtype)
+        } else {
+            write!(f, "dim=_, dtype={:?}", self.dtype)
+        }
+    }
+}
+
 /// Represents a casting error between rust types and numpy array.
 #[derive(Debug)]
 pub enum ErrorKind {
     /// Error for casting `PyArray` into `ArrayView` or `ArrayViewMut`
-    PyToRust { from: NpyDataType, to: NpyDataType },
+    PyToRust { from: ArrayDim, to: ArrayDim },
     /// Error for casting rust's `Vec` into numpy array.
     FromVec { dim1: usize, dim2: usize },
     /// Error in numpy -> numpy data conversion
-    PyToPy(Box<(ArrayFormat, ArrayFormat)>),
+    PyToPy(Box<(ArrayShape, ArrayShape)>),
 }
 
 impl ErrorKind {
-    pub(crate) fn to_rust(from: i32, to: NpyDataType) -> Self {
+    pub(crate) fn to_rust(
+        from_t: i32,
+        from_d: usize,
+        to_t: NpyDataType,
+        to_d: Option<usize>,
+    ) -> Self {
         ErrorKind::PyToRust {
-            from: NpyDataType::from_i32(from),
-            to,
+            from: ArrayDim {
+                dim: Some(from_d),
+                dtype: NpyDataType::from_i32(from_t),
+            },
+            to: ArrayDim {
+                dim: to_d,
+                dtype: to_t,
+            },
         }
     }
     pub(crate) fn dtype_cast<T: TypeNum, D>(from: &PyArray<T, D>, to: NpyDataType) -> Self {
-        let dims = from
-            .shape()
-            .into_iter()
-            .map(|&x| x)
-            .collect::<Vec<_>>()
-            .into_boxed_slice();
-        let from = ArrayFormat {
-            dims: dims.clone(),
-            dtype: T::npy_data_type(),
+        let from = ArrayShape::from_array(from);
+        let to = ArrayShape {
+            dims: from.dims.clone(),
+            dtype: to,
         };
-        let to = ArrayFormat { dims, dtype: to };
         ErrorKind::PyToPy(Box::new((from, to)))
     }
     pub(crate) fn dims_cast<T: TypeNum, D>(from: &PyArray<T, D>, to_dim: impl ToNpyDims) -> Self {
-        let dims_from = from
-            .shape()
-            .into_iter()
-            .map(|&x| x)
-            .collect::<Vec<_>>()
-            .into_boxed_slice();
-        let dims_to = to_dim
-            .slice()
-            .into_iter()
-            .map(|&x| x)
-            .collect::<Vec<_>>()
-            .into_boxed_slice();
-        let from = ArrayFormat {
-            dims: dims_from,
-            dtype: T::npy_data_type(),
-        };
-        let to = ArrayFormat {
-            dims: dims_to,
-            dtype: T::npy_data_type(),
-        };
+        let from = ArrayShape::from_array(from);
+        let to = ArrayShape::from_dims::<T, _>(to_dim);
         ErrorKind::PyToPy(Box::new((from, to)))
     }
 }
@@ -107,16 +134,16 @@ impl fmt::Display for ErrorKind {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             ErrorKind::PyToRust { from, to } => {
-                write!(f, "Cast failed: from={:?}, to={:?}", from, to)
+                write!(f, "Extraction failed:\n from=({}), to=({})", from, to)
             }
             ErrorKind::FromVec { dim1, dim2 } => write!(
                 f,
-                "Cast failed: Vec To PyArray: expect all dim {} but {} was found",
+                "Cast failed: Vec To PyArray:\n expect all dim {} but {} was found",
                 dim1, dim2
             ),
             ErrorKind::PyToPy(e) => write!(
                 f,
-                "Cast failed: from=ndarray({:?}), to=ndarray(dtype={:?})",
+                "Cast failed: from=ndarray({}), to=ndarray(dtype={})",
                 e.0, e.1,
             ),
         }
@@ -142,7 +169,7 @@ impl IntoPyErr for ErrorKind {
     fn into_pyerr_with<D: fmt::Display>(self, msg: impl FnOnce() -> D) -> PyErr {
         match self {
             ErrorKind::PyToRust { .. } | ErrorKind::FromVec { .. } | ErrorKind::PyToPy(_) => {
-                PyErr::new::<exc::TypeError, _>(format!("{} msg: {}", self, msg()))
+                PyErr::new::<exc::TypeError, _>(format!("{}\n context: {}", self, msg()))
             }
         }
     }
diff --git a/src/lib.rs b/src/lib.rs
@@ -53,7 +53,7 @@ pub use array::{
     PyArrayDyn,
 };
 pub use convert::{IntoPyArray, NpyIndex, ToNpyDims, ToPyArray};
-pub use error::{ArrayFormat, ErrorKind, IntoPyErr, IntoPyResult};
+pub use error::{ArrayShape, ErrorKind, IntoPyErr, IntoPyResult};
 pub use ndarray::{Ix1, Ix2, Ix3, Ix4, Ix5, Ix6, IxDyn};
 pub use npyffi::{PY_ARRAY_API, PY_UFUNC_API};
 pub use types::{c32, c64, NpyDataType, TypeNum};
diff --git a/src/slice_box.rs b/src/slice_box.rs
@@ -2,6 +2,8 @@ use crate::types::TypeNum;
 use pyo3::{ffi, typeob, types::PyObjectRef, PyObjectAlloc, Python, ToPyPointer};
 use std::os::raw::c_void;
 
+/// It's a memory store for IntoPyArray.
+/// See IntoPyArray's doc for what concretely this type is for.
 #[repr(C)]
 pub(crate) struct SliceBox<T> {
     ob_base: ffi::PyObject,
@@ -14,7 +16,7 @@ impl<T> SliceBox<T> {
         let type_ob = <Self as typeob::PyTypeInfo>::type_object() as *mut _;
         let base = ffi::_PyObject_New(type_ob);
         *base = ffi::PyObject_HEAD_INIT;
-        (*base).ob_type = <Self as typeob::PyTypeInfo>::type_object() as *mut _;
+        (*base).ob_type = type_ob;
         let self_ = base as *mut SliceBox<T>;
         (*self_).inner = Box::into_raw(box_);
         &*self_
@@ -28,12 +30,12 @@ impl<T> typeob::PyTypeInfo for SliceBox<T> {
     type Type = ();
     type BaseType = PyObjectRef;
     const NAME: &'static str = "SliceBox";
-    const DESCRIPTION: &'static str = "Memory store for PyArray made by IntoPyArray.";
+    const DESCRIPTION: &'static str = "Memory store for PyArray using rust's Box<[T]>.";
     const FLAGS: usize = 0;
-    const SIZE: usize = { Self::OFFSET as usize + std::mem::size_of::<Self>() + 0 + 0 };
+    const SIZE: usize = std::mem::size_of::<Self>();
     const OFFSET: isize = 0;
     #[inline]
-    unsafe fn type_object() -> &'static mut ::pyo3::ffi::PyTypeObject {
+    unsafe fn type_object() -> &'static mut ffi::PyTypeObject {
         static mut TYPE_OBJECT: ::pyo3::ffi::PyTypeObject = ::pyo3::ffi::PyTypeObject_INIT;
         &mut TYPE_OBJECT
     }
diff --git a/tests/array.rs b/tests/array.rs
