Rust errors (#53)

* adds better error messages to rust part
* adds better error messages to rust part
* verbose testing
* consume less memory in test_big_read
* better err messages
* Propagate change of error type to MATLAB API
* formatting

.github/workflows/python-module.yml

@@ -38,7 +38,7 @@ jobs:
           -v$(pwd):/app \
           -w/app/python \
           testing \
-          bash -c "\$PYBIN/pytest tests"
+          bash -c "\$PYBIN/pytest tests -v"
     - name: Check formatting
       run: |
         docker run \

c/Cargo.toml

@@ -4,7 +4,7 @@ version = "0.1.1"
 authors = ["Alessandro Motta <alessandro.motta@brain.mpg.de>"]
 
 [dependencies]
-lazy_static = "0.2"
+lazy_static = "1.4"
 
 [dependencies.wkwrap]
 path = "../rust"

c/src/lib.rs

@@ -29,7 +29,7 @@ fn as_log2(i: u8) -> Result<u8, &'static str> {
     }
 }
 
-fn from_header(header_ptr: *const Header) -> Result<wkw::Header, &'static str> {
+fn from_header(header_ptr: *const Header) -> Result<wkw::Header, String> {
     assert!(!header_ptr.is_null());
 
     let c_header = unsafe { &*header_ptr };
@@ -38,7 +38,7 @@ fn from_header(header_ptr: *const Header) -> Result<wkw::Header, &'static str> {
         1 => wkw::BlockType::Raw,
         2 => wkw::BlockType::LZ4,
         3 => wkw::BlockType::LZ4HC,
-        _ => return Err("Block type is invalid"),
+        other => return Err(format!("Block type '{}' is invalid", other)),
     };
 
     let voxel_type = match c_header.voxel_type {
@@ -52,7 +52,7 @@ fn from_header(header_ptr: *const Header) -> Result<wkw::Header, &'static str> {
         8 => wkw::VoxelType::I16,
         9 => wkw::VoxelType::I32,
         10 => wkw::VoxelType::I64,
-        _ => return Err("Voxel type is invalid"),
+        other => return Err(format!("Voxel type '{}' is invalid", other)),
     };
 
     let block_len_log2 = as_log2(c_header.block_len)?;
@@ -70,11 +70,11 @@ fn from_header(header_ptr: *const Header) -> Result<wkw::Header, &'static str> {
     })
 }
 
-fn check_return<T>(ret: Result<T, &str>) -> c_int {
+fn check_return<T>(ret: Result<T, String>) -> c_int {
     match ret {
         Ok(_) => 0,
         Err(msg) => {
-            set_last_error_msg(msg);
+            set_last_error_msg(&msg);
             1
         }
     }
@@ -107,7 +107,7 @@ pub extern "C" fn dataset_open(root_ptr: *const c_char) -> *const Dataset {
             unsafe { std::mem::transmute(dataset_ptr) }
         }
         Err(msg) => {
-            set_last_error_msg(msg);
+            set_last_error_msg(&msg);
             std::ptr::null::<Dataset>()
         }
     }
@@ -161,7 +161,7 @@ pub extern "C" fn dataset_create(
             unsafe { std::mem::transmute(dataset_ptr) }
         }
         Err(msg) => {
-            set_last_error_msg(msg);
+            set_last_error_msg(&msg);
             std::ptr::null::<Dataset>()
         }
     }

matlab/rust/wkw_compress/src/lib.rs

@@ -10,7 +10,7 @@ use std::path::Path;
 mex_function!(_nlhs, _lhs, nrhs, rhs, {
     let rhs = match nrhs == 2 {
         true => slice::from_raw_parts(rhs, nrhs as usize),
-        false => return Err("Invalid number of input arguments")
+        false => return Err("Invalid number of input arguments".to_string())
     };
 
     let src_path = Path::new(mx_array_to_str(rhs[0])?);

matlab/rust/wkw_init/src/lib.rs

@@ -9,7 +9,7 @@ use std::path::Path;
 unsafe fn new(nrhs: c_int, rhs: *const MxArray) -> Result<()> {
     let rhs = match nrhs == 5 {
         true => slice::from_raw_parts(rhs, nrhs as usize),
-        false => return Err("Invalid number of input arguments")
+        false => return Err("Invalid number of input arguments".to_string())
     };
 
     let wkw_path = Path::new(mx_array_to_str(rhs[0])?);
@@ -46,7 +46,7 @@ unsafe fn new(nrhs: c_int, rhs: *const MxArray) -> Result<()> {
 unsafe fn compress(nrhs: c_int, rhs: *const MxArray) -> Result<()> {
     let rhs = match nrhs == 2 {
         true => slice::from_raw_parts(rhs, nrhs as usize),
-        false => return Err("Invalid number of input arguments")
+        false => return Err("Invalid number of input arguments".to_string())
     };
 
     let src_path = Path::new(mx_array_to_str(rhs[0])?);
@@ -60,13 +60,13 @@ unsafe fn compress(nrhs: c_int, rhs: *const MxArray) -> Result<()> {
 
 mex_function!(_nlhs, _lhs, nrhs, rhs, {
     let command = match nrhs < 1 {
-        true => Err("Not enough input arguments"),
+        true => Err("Not enough input arguments".to_string()),
         false => mx_array_to_str(*rhs)
     }?;
 
     match command {
         "new" => new(nrhs - 1, rhs.offset(1)),
         "compress" => compress(nrhs - 1, rhs.offset(1)),
-        _ => Err("Unknown command")
+        _ => Err("Unknown command".to_string())
     }
 });

matlab/rust/wkw_load/src/lib.rs

@@ -10,12 +10,12 @@ use std::path::Path;
 mex_function!(nlhs, lhs, nrhs, rhs, {
     let rhs = match nrhs == 2 {
         true => slice::from_raw_parts(rhs, nrhs as usize),
-        false => return Err("Invalid number of input arguments")
+        false => return Err("Invalid number of input arguments".to_string())
     };
 
     let lhs = match nlhs == 1 {
         true => slice::from_raw_parts_mut(lhs, nlhs as usize),
-        false => return Err("Invalid number of output arguments")
+        false => return Err("Invalid number of output arguments".to_string())
     };
 
     let wkw_path = mx_array_to_str(rhs[0])?;

matlab/rust/wkw_mex/src/macros.rs

@@ -51,7 +51,7 @@ pub extern fn mexFunction(
 
     match unsafe { body(nlhs, plhs, nrhs, prhs) } {
         Ok(_) => (),
-        Err(msg) => die(msg)
+        Err(msg) => die(&msg)
     }
 }
     }

matlab/rust/wkw_mex/src/util.rs

@@ -4,16 +4,16 @@ use std;
 use std::slice;
 use std::ffi::CStr;
 
-pub type Result<T> = std::result::Result<T, &'static str>;
+pub type Result<T> = std::result::Result<T, String>;
 
 pub fn as_nat(f: f64) -> Result<u64> {
     if f <= 0.0 {
-        return Err("Input must be positive");
+        return Err("Input must be positive".to_string())
     }
 
     match f % 1.0 == 0.0 {
         true => Ok(f as u64),
-        false => Err("Input must be an integer")
+        false => Err("Input must be an integer".to_string())
     }
 }
 
@@ -22,7 +22,7 @@ pub fn as_log2(f: f64) -> Result<u8> {
 
     match i & (i - 1) == 0 {
         true => Ok(i.trailing_zeros() as u8),
-        false => Err("Input must be a power of two")
+        false => Err("Input must be a power of two".to_string())
     }
 }
 
@@ -38,36 +38,36 @@ pub fn str_slice_to_mx_class_id(class_id: &str) -> Result<MxClassId> {
         "int16"  => Ok(MxClassId::Int16),
         "int32"  => Ok(MxClassId::Int32),
         "int64"  => Ok(MxClassId::Int64),
-        _        => Err("Unknown MxClassId name")
+        _        => Err("Unknown MxClassId name".to_string())
     }
 }
 
 pub fn mx_array_to_str<'a>(pm: MxArray) -> Result<&'a str> {
     let pm_ptr = unsafe { mxArrayToUTF8String(pm) };
 
     if pm_ptr.is_null() {
-        return Err("mxArrayToUTF8String returned null")
+        return Err("mxArrayToUTF8String returned null".to_string())
     }
 
     let pm_cstr = unsafe { CStr::from_ptr(pm_ptr) };
 
     match pm_cstr.to_str() {
         Ok(pm_str) => Ok(pm_str),
-        Err(_) => Err("mxArray contains invalid UTF-8 data")
+        Err(_) => Err("mxArray contains invalid UTF-8 data".to_string())
     }
 }
 
 pub fn mx_array_to_f64_slice<'a>(pm: MxArray) -> Result<&'a [f64]> {
     unsafe {
-        if !mxIsDouble(pm) { return Err("MxArray is not of class \"double\"") };
-        if mxIsComplex(pm) { return Err("MxArray is complex") };
+        if !mxIsDouble(pm) { return Err("MxArray is not of class \"double\"".to_string()) };
+        if mxIsComplex(pm) { return Err("MxArray is complex".to_string()) };
     }
 
     let pm_numel = unsafe { mxGetNumberOfElements(pm) };
     let pm_ptr = unsafe { mxGetPr(pm) };
 
     match pm_ptr.is_null() {
-        true => Err("MxArray does not contain real values"),
+        true => Err("MxArray does not contain real values".to_string()),
         false => Ok(unsafe { slice::from_raw_parts(pm_ptr, pm_numel) })
     }
 }
@@ -77,7 +77,7 @@ pub fn mx_array_to_f64(pm: MxArray) -> Result<f64> {
 
     match pm_slice.len() {
         1 => Ok(pm_slice[0]),
-        _ => Err("MxArray contains an invalid number of doubles")
+        _ => Err("MxArray contains an invalid number of doubles".to_string())
     }
 }
 
@@ -87,9 +87,9 @@ pub fn mx_array_to_u8_slice<'a>(pm: MxArray) -> Result<&'a [u8]> {
     let data = unsafe { mxGetData(pm) } as *const u8;
 
     if elem_size == 0 {
-        Err("Failed to determine element size")
+        Err("Failed to determine element size".to_string())
     } else if data.is_null() {
-        Err("Data pointer is null")
+        Err("Data pointer is null".to_string())
     } else {
         Ok(unsafe { slice::from_raw_parts(data, numel * elem_size) })
     }
@@ -101,9 +101,9 @@ pub fn mx_array_mut_to_u8_slice_mut<'a>(pm: MxArrayMut) -> Result<&'a mut [u8]>
     let data = unsafe { mxGetData(pm) } as *mut u8;
 
     if elem_size == 0 {
-        Err("Failed to determine element size")
+        Err("Failed to determine element size".to_string())
     } else if data.is_null() {
-        Err("Data pointer is null")
+        Err("Data pointer is null".to_string())
     } else {
         Ok(unsafe { slice::from_raw_parts_mut(data, numel * elem_size) })
     }
@@ -130,7 +130,7 @@ pub fn create_numeric_array(
     };
 
     match arr.is_null() {
-        true => Err("Failed to create uninitialized numeric array"),
+        true => Err("Failed to create uninitialized numeric array".to_string()),
         false => Ok(arr)
     }
 }
@@ -139,7 +139,7 @@ pub fn malloc(n: usize) -> Result<&'static mut [u8]> {
     let ptr = unsafe { mxMalloc(n as MwSize) } as *mut u8;
 
     match ptr.is_null() {
-        true => Err("Failed to allocate memory"),
+        true => Err("Failed to allocate memory".to_string()),
         false => Ok(unsafe { slice::from_raw_parts_mut(ptr, n) })
     }
 }

matlab/rust/wkw_mex/src/wkw.rs

@@ -5,11 +5,11 @@ use ::wkwrap;
 fn f64_slice_to_wkwrap_vec(buf: &[f64]) -> Result<wkwrap::Vec3> {
     match buf.len() == 3 {
         true => Ok(wkwrap::Vec3 {
-            x: as_nat(buf[0]).or(Err("Invalid X value"))? as u32,
-            y: as_nat(buf[1]).or(Err("Invalid Y value"))? as u32,
-            z: as_nat(buf[2]).or(Err("Invalid Z value"))? as u32
+            x: as_nat(buf[0]).or(Err("Invalid X value".to_string()))? as u32,
+            y: as_nat(buf[1]).or(Err("Invalid Y value".to_string()))? as u32,
+            z: as_nat(buf[2]).or(Err("Invalid Z value".to_string()))? as u32
         }),
-        false => Err("Size mismatch")
+        false => Err("Size mismatch".to_string())
     }
 }
 
@@ -18,12 +18,12 @@ pub fn mx_array_to_wkwrap_box(pm: MxArray) -> Result<wkwrap::Box3> {
 
     // verify shape of array
     if mx_array_size_to_usize_slice(pm) != &[3, 2] {
-        return Err("Bounding box has invalid shape");
+        return Err("Bounding box has invalid shape".to_string());
     }
 
     wkwrap::Box3::new(
-        f64_slice_to_wkwrap_vec(&buf[0..3]).or(Err("Invalid lower bound"))? - 1,
-        f64_slice_to_wkwrap_vec(&buf[3..6]).or(Err("Invalid upper bound"))?
+        f64_slice_to_wkwrap_vec(&buf[0..3]).or(Err("Invalid lower bound".to_string()))? - 1,
+        f64_slice_to_wkwrap_vec(&buf[3..6]).or(Err("Invalid upper bound".to_string()))?
     )
 }
 
@@ -49,7 +49,7 @@ pub fn mx_class_id_to_voxel_type(class_id: MxClassId) -> Result<wkwrap::VoxelTyp
         MxClassId::Int16  => Ok(wkwrap::VoxelType::I16),
         MxClassId::Int32  => Ok(wkwrap::VoxelType::I32),
         MxClassId::Int64  => Ok(wkwrap::VoxelType::I64),
-        _                 => Err("Unknown MxClassId")
+        _                 => Err("Unknown MxClassId".to_string())
     }
 }
 
@@ -81,7 +81,7 @@ pub fn mx_array_mut_to_wkwrap_mat<'a>(
 
     // check number of input dimensions
     if mx_size_len > if is_multi_channel { 4 } else { 3 } {
-        return Err("Data array has too many dimensions");
+        return Err("Data array has too many dimensions".to_string());
     }
 
     let mut size = [1usize; 4];

matlab/rust/wkw_save/src/lib.rs

@@ -10,7 +10,7 @@ use std::path::Path;
 mex_function!(_nlhs, _lhs, nrhs, rhs, {
     let rhs = match nrhs == 3 {
         true => slice::from_raw_parts(rhs, nrhs as usize),
-        false => return Err("Invalid number of input arguments")
+        false => return Err("Invalid number of input arguments".to_string())
     };
 
     // path to root

python/tests/test_wkw.py

@@ -1,7 +1,7 @@
 import wkw
 import numpy as np
 import shutil
-from os import path
+from os import path, makedirs
 import pytest
 
 POSITION = (0, 0, 0)
@@ -291,18 +291,38 @@ def test_multiple_writes_and_reads():
 def test_big_read():
     data = np.ones((10, 10, 764), order="C", dtype=np.uint8)
     offset = np.array([0, 0, 640])
-    bottom = (2000, 2000, 2000)
-    mem_buffer = np.zeros(bottom, dtype=np.uint8, order="F")
+    bottom = (1200, 2000, 2000)
 
     with wkw.Dataset.create("tests/tmp", wkw.Header(np.uint8)) as dataset:
         dataset.write(offset, data)
-        mem_buffer[
-            offset[0] : offset[0] + data.shape[0],
-            offset[1] : offset[1] + data.shape[1],
-            offset[2] : offset[2] + data.shape[2],
-        ] = data
-        read_data = dataset.read((0, 0, 0), bottom)
-        assert np.all(read_data == mem_buffer)
+        read_data = dataset.read((0, 0, 0), bottom)[0]
+        assert np.all(
+            read_data[
+                offset[0] : (offset[0] + data.shape[0]),
+                offset[1] : (offset[1] + data.shape[1]),
+                offset[2] : (offset[2] + data.shape[2]),
+            ]
+            == 1
+        )
+        assert np.count_nonzero(read_data[: offset[0], :, :]) == 0
+        assert np.count_nonzero(read_data[offset[0] + data.shape[0] :, :, :]) == 0
+        assert np.count_nonzero(read_data[:, : offset[1], :]) == 0
+        assert np.count_nonzero(read_data[:, offset[1] + data.shape[1] :, :]) == 0
+        assert np.count_nonzero(read_data[:, :, : offset[2]]) == 0
+        assert np.count_nonzero(read_data[:, :, offset[2] + data.shape[2] :]) == 0
+
+
+def test_invalid_dataset():
+    with pytest.raises(wkw.wkw.WKWException) as excinfo:
+        with wkw.Dataset.open("/path/does/not/exist") as dataset:
+            pass
+    print(excinfo.value)
+
+    with pytest.raises(wkw.wkw.WKWException) as excinfo:
+        makedirs("tests/tmp/exists", exist_ok=True)
+        with wkw.Dataset.open("tests/tmp/exists") as dataset:
+            pass
+    print(excinfo.value)
 
 
 def generate_test_data(dtype, size=SIZE, order="C"):