From 9e0cb250c995caf18182f8bfbe10ae967dc31210 Mon Sep 17 00:00:00 2001 From: Cian Hughes Date: Wed, 8 May 2024 17:05:28 +0100 Subject: [PATCH] Improved error handling Significantly improved error handling. Rather than panicking and crashing we should be propagating all errors up to python that reasonably can be. This should make the library significantly easier to call from python in future. --- Cargo.toml | 1 + src/lib.rs | 45 +++++++++++++++++++++++------ src/rust_fn/mod.rs | 71 ++++++++++++++++++++++++++++++++-------------- 3 files changed, 87 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3fb7d2b..d15ec8d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ ndarray-csv = "*" numpy = "*" pyo3 = { version = "0.18.1", features = ["extension-module"] } rayon = "*" +thiserror = "1.0.60" [dev-dependencies] anyhow = "1.0.83" diff --git a/src/lib.rs b/src/lib.rs index 5a7c853..637d457 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,31 +1,58 @@ use numpy::{IntoPyArray, PyArray2}; -use pyo3::prelude::{pymodule, PyModule, PyResult, Python}; +use pyo3::exceptions; +use pyo3::prelude::{pymodule, PyErr, PyModule, PyResult, Python}; use pyo3::types::{PyList, PyString}; use std::path::Path; pub mod rust_fn; +impl From for PyErr { + fn from(err: rust_fn::ReadError) -> Self { + match err { + rust_fn::ReadError::Glob(e) => { + PyErr::new::(format!("{}", e)) + } + rust_fn::ReadError::GlobPattern(e) => { + PyErr::new::(format!("{}", e)) + } + rust_fn::ReadError::Io(e) => PyErr::new::(format!("{}", e)), + rust_fn::ReadError::NdarrayCSV(e) => { + PyErr::new::(format!("{}", e)) + } + rust_fn::ReadError::ParseFloatError(e) => { + PyErr::new::(format!("{}", e)) + } + rust_fn::ReadError::MiscError(e) => { + PyErr::new::(format!("{}", e)) + } + } + } +} + #[pymodule] fn read_aconity_layers(_py: Python<'_>, m: &PyModule) -> PyResult<()> { #[pyfn(m)] - fn read_layers<'py>(py: Python<'py>, folder: &PyString) -> &'py PyArray2 { - rust_fn::read_layers(folder.to_str().unwrap()).into_pyarray(py) + fn read_layers<'py>(py: Python<'py>, folder: &PyString) -> PyResult<&'py PyArray2> { + Ok(rust_fn::read_layers(folder.to_str().unwrap())?.into_pyarray(py)) } #[pyfn(m)] - fn read_selected_layers<'py>(py: Python<'py>, file_list: &PyList) -> &'py PyArray2 { - rust_fn::read_selected_layers( + fn read_selected_layers<'py>( + py: Python<'py>, + file_list: &PyList, + ) -> PyResult<&'py PyArray2> { + Ok(rust_fn::read_selected_layers( file_list .iter() .map(|x| Path::new(&(*x).str().unwrap().to_string()).to_path_buf()) .collect(), - ) - .into_pyarray(py) + )? + .into_pyarray(py)) } #[pyfn(m)] - fn read_layer<'py>(py: Python<'py>, file: &PyString) -> &'py PyArray2 { - rust_fn::read_layer(file.to_str().unwrap()).into_pyarray(py) + fn read_layer<'py>(py: Python<'py>, file: &PyString) -> PyResult<&'py PyArray2> { + Ok(rust_fn::read_layer(file.to_str().unwrap())?.into_pyarray(py)) } Ok(()) diff --git a/src/rust_fn/mod.rs b/src/rust_fn/mod.rs index 5597784..1a26459 100644 --- a/src/rust_fn/mod.rs +++ b/src/rust_fn/mod.rs @@ -1,20 +1,45 @@ use csv::ReaderBuilder; -use glob::{glob, GlobError}; +use glob::glob; use indicatif::ProgressBar; use ndarray::{concatenate, Array2, ArrayView2, Axis, Slice}; use ndarray_csv::Array2Reader; use rayon::prelude::*; -use std::error::Error; use std::fs::File; use std::path::{Path, PathBuf}; +use thiserror::Error; -pub fn read_layers(folder: &str) -> Array2 { +#[derive(Error, Debug)] +pub enum ReadError { + #[error("Glob error: {0}")] + Glob(#[from] glob::GlobError), + + #[error("Glob pattern error: {0}")] + GlobPattern(#[from] glob::PatternError), + + #[error("IO error: {0}")] + Io(#[from] std::io::Error), + + #[error("NdarrayCSV error: {0}")] + NdarrayCSV(#[from] ndarray_csv::ReadError), + + #[error("Float parse error: {0}")] + ParseFloatError(#[from] std::num::ParseFloatError), + + #[error("Miscellaneous Error")] + MiscError(String), +} + +pub type Result = std::result::Result; + +pub fn read_layers(folder: &str) -> Result> { let glob_string: String = folder.to_owned() + "/*.pcd"; - let mut glob_iterator: Vec = glob(glob_string.as_str()) - .expect("Files not found!") - .collect::, GlobError>>() - .unwrap(); - glob_iterator.par_sort_unstable_by(|a, b| get_z(a).partial_cmp(&get_z(b)).unwrap()); + let mut glob_iterator = glob(glob_string.as_str())? + .collect::, glob::GlobError>>()?; + glob_iterator.par_sort_unstable_by(|a, b| { + let az = get_z(&a).expect("Filename parsing failed."); + let bz = get_z(&b).expect("Filename parsing failed."); + az.partial_cmp(&bz).expect("Filename sorting failed") + }); let len: usize = glob_iterator.len(); let bar = ProgressBar::new(len as u64); let mut arrays: Vec> = vec![Array2::::zeros((0, 0)); len]; @@ -62,10 +87,10 @@ pub fn read_layers(folder: &str) -> Array2 { out_array.column_mut(0).par_map_inplace(correct_x); out_array.column_mut(1).par_map_inplace(correct_y); - out_array + Ok(out_array) } -pub fn read_selected_layers(file_list: Vec) -> Array2 { +pub fn read_selected_layers(file_list: Vec) -> Result> { let len: usize = file_list.len(); let bar = ProgressBar::new(len as u64); let mut arrays: Vec> = vec![Array2::::zeros((0, 0)); len]; @@ -113,10 +138,10 @@ pub fn read_selected_layers(file_list: Vec) -> Array2 { out_array.column_mut(0).par_map_inplace(correct_x); out_array.column_mut(1).par_map_inplace(correct_y); - out_array + Ok(out_array) } -pub fn read_layer(file: &str) -> Array2 { +pub fn read_layer(file: &str) -> Result> { let (array, z, z_len) = read_file(Path::new(file).to_path_buf()).unwrap(); let z_array: Array2 = Array2::from_elem((z_len, 1), z); let z_array_view: ArrayView2 = z_array.view(); @@ -127,11 +152,11 @@ pub fn read_layer(file: &str) -> Array2 { out_array.column_mut(0).par_map_inplace(correct_x); out_array.column_mut(1).par_map_inplace(correct_y); - out_array + Ok(out_array) } -pub fn read_file(filepath: PathBuf) -> Result<(Array2, f64, usize), Box> { - let z: f64 = get_z(&filepath); +pub fn read_file(filepath: PathBuf) -> Result<(Array2, f64, usize)> { + let z: f64 = get_z(&filepath)?; let file = File::open(filepath)?; let mut rdr = ReaderBuilder::new() .has_headers(false) @@ -143,14 +168,18 @@ pub fn read_file(filepath: PathBuf) -> Result<(Array2, f64, usize), Box f64 { - filepath +pub fn get_z(filepath: &PathBuf) -> Result { + Ok(filepath .file_stem() - .unwrap() + .ok_or(ReadError::MiscError(format!( + "No file stem found for file {}", + filepath.to_str().ok_or(ReadError::MiscError( + "No filepath found... if this happens something very weird has happened".to_owned() + ))? + )))? .to_str() - .unwrap() - .parse::() - .unwrap() + .ok_or(ReadError::MiscError("Failed to parse filename".to_owned()))? + .parse::()?) } pub fn correct_x(x: &mut f64) -> () {