Improve CSV loading error reporting

Author: cmccomb

src/utils/io.rs

@@ -1,6 +1,6 @@
 //! I/O utilities.
 
-use csv::ReaderBuilder;
+use csv::{ReaderBuilder, StringRecord};
 use smartcore::linalg::basic::matrix::DenseMatrix;
 use std::error::Error;
 use std::fmt;
@@ -49,6 +49,8 @@ impl Error for CsvError {
 ///
 /// Returns an error if the file cannot be read, a value fails to parse into
 /// `f64`, or the rows have inconsistent lengths.
+/// Row numbers mentioned in error messages are one-based and refer to data rows,
+/// excluding the header.
 ///
 /// # Examples
 ///
@@ -66,29 +68,19 @@ impl Error for CsvError {
 /// # }
 /// ```
 pub fn load_csv_features<P: AsRef<Path>>(path: P) -> Result<DenseMatrix<f64>, CsvError> {
-    let file = File::open(path.as_ref()).map_err(CsvError::Io)?;
-    let mut reader = ReaderBuilder::new()
-        .has_headers(true)
-        .flexible(true)
-        .from_reader(file);
-
+    let mut reader = build_csv_reader(path.as_ref())?;
     let mut features: Vec<Vec<f64>> = Vec::new();
+    let mut expected_width: Option<usize> = None;
 
-    for result in reader.records() {
+    for (row_idx, result) in reader.records().enumerate() {
         let record = result.map_err(|e| CsvError::Parse(Box::new(e)))?;
-        let row = record
-            .iter()
-            .map(|v| {
-                v.parse::<f64>()
-                    .map_err(|e: ParseFloatError| CsvError::Parse(Box::new(e)))
-            })
-            .collect::<Result<Vec<_>, _>>()?;
+        let row = parse_feature_row(&record, row_idx)?;
+        ensure_consistent_width(&row, row_idx, &mut expected_width)?;
         features.push(row);
     }
 
-    let expected = features.first().map_or(0, Vec::len);
-    if features.iter().any(|r| r.len() != expected) {
-        return Err(CsvError::Shape("inconsistent row lengths".to_string()));
+    if features.is_empty() {
+        return Err(CsvError::Shape("no rows found".to_string()));
     }
 
     let matrix = DenseMatrix::from_2d_vec(&features).map_err(|e| CsvError::Shape(e.to_string()))?;
@@ -106,6 +98,8 @@ pub fn load_csv_features<P: AsRef<Path>>(path: P) -> Result<DenseMatrix<f64>, Cs
 ///
 /// Returns an error if the file cannot be read, a value fails to parse into
 /// `f64`, or the rows have inconsistent lengths.
+/// Row numbers mentioned in error messages are one-based and refer to data rows,
+/// excluding the header.
 ///
 /// # Examples
 ///
@@ -123,40 +117,159 @@ pub fn load_labeled_csv<P: AsRef<Path>>(
     path: P,
     target_col: usize,
 ) -> Result<(DenseMatrix<f64>, Vec<f64>), CsvError> {
-    let file = File::open(path.as_ref()).map_err(CsvError::Io)?;
-    let mut reader = ReaderBuilder::new()
-        .has_headers(true)
-        .flexible(true)
-        .from_reader(file);
-
+    let mut reader = build_csv_reader(path.as_ref())?;
     let mut features: Vec<Vec<f64>> = Vec::new();
     let mut targets: Vec<f64> = Vec::new();
+    let mut expected_width: Option<usize> = None;
 
-    for result in reader.records() {
+    for (row_idx, result) in reader.records().enumerate() {
         let record = result.map_err(|e| CsvError::Parse(Box::new(e)))?;
-        let mut row: Vec<f64> = Vec::new();
-        for (idx, field) in record.iter().enumerate() {
-            let value: f64 = field
-                .parse::<f64>()
-                .map_err(|e: ParseFloatError| CsvError::Parse(Box::new(e)))?;
-            if idx == target_col {
-                targets.push(value);
-            } else {
-                row.push(value);
-            }
-        }
+        let (row, target) = parse_labeled_row(&record, row_idx, target_col)?;
+        ensure_consistent_width(&row, row_idx, &mut expected_width)?;
+        targets.push(target);
         features.push(row);
     }
 
-    if targets.len() != features.len() {
-        return Err(CsvError::Shape("inconsistent row lengths".to_string()));
-    }
-
-    let expected = features.first().map_or(0, Vec::len);
-    if features.iter().any(|r| r.len() != expected) {
-        return Err(CsvError::Shape("inconsistent row lengths".to_string()));
+    if features.is_empty() {
+        return Err(CsvError::Shape("no rows found".to_string()));
     }
 
     let matrix = DenseMatrix::from_2d_vec(&features).map_err(|e| CsvError::Shape(e.to_string()))?;
     Ok((matrix, targets))
 }
+
+fn build_csv_reader(path: &Path) -> Result<csv::Reader<File>, CsvError> {
+    let file = File::open(path).map_err(CsvError::Io)?;
+    Ok(ReaderBuilder::new()
+        .has_headers(true)
+        .flexible(true)
+        .from_reader(file))
+}
+
+fn parse_feature_row(record: &StringRecord, row_idx: usize) -> Result<Vec<f64>, CsvError> {
+    if record.is_empty() {
+        return Err(CsvError::Shape(format!(
+            "row {}: expected at least one column",
+            row_idx + 1
+        )));
+    }
+
+    record
+        .iter()
+        .enumerate()
+        .map(|(col_idx, value)| parse_numeric_field(value, row_idx, col_idx))
+        .collect()
+}
+
+fn parse_labeled_row(
+    record: &StringRecord,
+    row_idx: usize,
+    target_col: usize,
+) -> Result<(Vec<f64>, f64), CsvError> {
+    if record.len() <= target_col {
+        return Err(CsvError::Shape(format!(
+            "row {}: target column index {} out of bounds (row has {} columns)",
+            row_idx + 1,
+            target_col,
+            record.len()
+        )));
+    }
+
+    if record.len() <= 1 {
+        return Err(CsvError::Shape(format!(
+            "row {}: expected at least one feature column in addition to the target",
+            row_idx + 1
+        )));
+    }
+
+    let mut target = None;
+    let mut row = Vec::with_capacity(record.len() - 1);
+
+    for (col_idx, value) in record.iter().enumerate() {
+        let parsed = parse_numeric_field(value, row_idx, col_idx)?;
+        if col_idx == target_col {
+            target = Some(parsed);
+        } else {
+            row.push(parsed);
+        }
+    }
+
+    match target {
+        Some(target_value) => Ok((row, target_value)),
+        None => Err(CsvError::Shape(format!(
+            "row {}: missing target column {}",
+            row_idx + 1,
+            target_col
+        ))),
+    }
+}
+
+fn parse_numeric_field(value: &str, row_idx: usize, col_idx: usize) -> Result<f64, CsvError> {
+    value.parse::<f64>().map_err(|err: ParseFloatError| {
+        CsvError::Parse(Box::new(FloatParseError::new(
+            row_idx + 1,
+            col_idx + 1,
+            err,
+        )))
+    })
+}
+
+fn ensure_consistent_width(
+    row: &[f64],
+    row_idx: usize,
+    expected_width: &mut Option<usize>,
+) -> Result<(), CsvError> {
+    if row.is_empty() {
+        return Err(CsvError::Shape(format!(
+            "row {}: expected at least one column",
+            row_idx + 1
+        )));
+    }
+
+    match expected_width {
+        Some(width) if row.len() != *width => Err(CsvError::Shape(format!(
+            "row {}: expected {} columns but found {}",
+            row_idx + 1,
+            width,
+            row.len()
+        ))),
+        Some(_) => Ok(()),
+        None => {
+            *expected_width = Some(row.len());
+            Ok(())
+        }
+    }
+}
+
+#[derive(Debug)]
+struct FloatParseError {
+    row: usize,
+    column: usize,
+    source: ParseFloatError,
+}
+
+impl FloatParseError {
+    fn new(row: usize, column: usize, source: ParseFloatError) -> Self {
+        Self {
+            row,
+            column,
+            source,
+        }
+    }
+}
+
+impl fmt::Display for FloatParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "failed to parse float at row {}, column {}: {}",
+            self.row, self.column, self.source
+        )
+    }
+}
+
+impl Error for FloatParseError {
+    fn source(&self) -> Option<&(dyn Error + 'static)> {
+        Some(&self.source)
+    }
+}

tests/load_csv_features.rs

@@ -18,11 +18,26 @@ fn errors_on_bad_path() {
 #[test]
 fn errors_on_non_numeric() {
     let err = load_csv_features("tests/fixtures/non_numeric_features.csv").unwrap_err();
-    assert!(matches!(err, CsvError::Parse(_)));
+    match err {
+        CsvError::Parse(parse_err) => {
+            let message = parse_err.to_string();
+            assert!(message.contains("row 2"), "unexpected message: {message}");
+            assert!(
+                message.contains("column 1"),
+                "unexpected message: {message}"
+            );
+        }
+        other => panic!("expected parse error, got {other}"),
+    }
 }
 
 #[test]
 fn errors_on_inconsistent_rows() {
     let err = load_csv_features("tests/fixtures/inconsistent_features.csv").unwrap_err();
-    assert!(matches!(err, CsvError::Shape(_)));
+    match err {
+        CsvError::Shape(message) => {
+            assert!(message.contains("row 2"), "unexpected message: {message}");
+        }
+        other => panic!("expected shape error, got {other}"),
+    }
 }

tests/load_labeled_csv.rs

@@ -22,11 +22,45 @@ fn errors_on_bad_path() {
 #[test]
 fn errors_on_non_numeric() {
     let err = load_labeled_csv("tests/fixtures/non_numeric_labeled.csv", 2).unwrap_err();
-    assert!(matches!(err, CsvError::Parse(_)));
+    match err {
+        CsvError::Parse(parse_err) => {
+            let message = parse_err.to_string();
+            assert!(message.contains("row 2"), "unexpected message: {message}");
+            assert!(
+                message.contains("column 2"),
+                "unexpected message: {message}"
+            );
+        }
+        other => panic!("expected parse error, got {other}"),
+    }
 }
 
 #[test]
 fn errors_on_inconsistent_rows() {
     let err = load_labeled_csv("tests/fixtures/inconsistent_labeled.csv", 2).unwrap_err();
-    assert!(matches!(err, CsvError::Shape(_)));
+    match err {
+        CsvError::Shape(message) => {
+            assert!(message.contains("row 2"), "unexpected message: {message}");
+            assert!(
+                message.contains("out of bounds"),
+                "unexpected message: {message}"
+            );
+        }
+        other => panic!("expected shape error, got {other}"),
+    }
+}
+
+#[test]
+fn errors_when_target_column_out_of_range() {
+    let err = load_labeled_csv("tests/fixtures/supervised_sample.csv", 5).unwrap_err();
+    match err {
+        CsvError::Shape(message) => {
+            assert!(message.contains("row 1"), "unexpected message: {message}");
+            assert!(
+                message.contains("out of bounds"),
+                "unexpected message: {message}"
+            );
+        }
+        other => panic!("expected shape error, got {other}"),
+    }
 }