CSV Error Refactor (#216)

Hey @nyurik , I made an error module for `csv` in response to #138 . Let me know if you think any changes need to be made. --------- Co-authored-by: Michael Kirk <[email protected]>
georust · Sep 18, 2024 · 142c080 · 142c080
1 parent 6259e88
commit 142c080
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 21 deletions.
diff --git a/geozero/src/csv/csv_error.rs b/geozero/src/csv/csv_error.rs
@@ -0,0 +1,10 @@
+use thiserror::Error;
+
+/// CSV Error type.
+#[derive(Error, Debug)]
+pub enum CsvError {
+    #[error("column not found or null")]
+    ColumnNotFound,
+    #[error("error parsing to WKT `{0}`")]
+    WktError(&'static str),
+}
diff --git a/geozero/src/csv/csv_reader.rs b/geozero/src/csv/csv_reader.rs
@@ -1,6 +1,8 @@
 use crate::error::{GeozeroError, Result};
 use crate::{ColumnValue, FeatureProcessor, GeomProcessor, GeozeroDatasource, GeozeroGeometry};
 
+use crate::csv::csv_error::CsvError;
+
 use std::io::Read;
 use std::str::FromStr;
 
@@ -109,17 +111,14 @@ pub fn process_csv_geom(
     let geometry_idx = headers
         .iter()
         .position(|f| f == geometry_column)
-        .ok_or(GeozeroError::ColumnNotFound)?;
+        .ok_or(CsvError::ColumnNotFound)?;
 
     let mut collection_started = false;
 
     for (record_idx, record) in reader.into_records().enumerate() {
         let record = record?;
-        let geometry_field = record
-            .get(geometry_idx)
-            .ok_or(GeozeroError::ColumnNotFound)?;
-        let wkt = wkt::Wkt::from_str(geometry_field)
-            .map_err(|e| GeozeroError::Geometry(e.to_string()))?;
+        let geometry_field = record.get(geometry_idx).ok_or(CsvError::ColumnNotFound)?;
+        let wkt = wkt::Wkt::from_str(geometry_field).map_err(CsvError::WktError)?;
 
         // We don't know how many lines are in the file, so we dont' know the size of the geometry collection,
         // but at this point we *do* know that it's non-zero. Currently there aren't any other significant
@@ -132,12 +131,13 @@ pub fn process_csv_geom(
             processor.geometrycollection_begin(1, 0)?;
         }
 
-        crate::wkt::wkt_reader::process_wkt_geom_n(&wkt, record_idx, processor).map_err(|e| {
-            // +2 to start at line 1 and to account for the header row
-            let line = record_idx + 2;
-            log::warn!("line {line}: invalid WKT: '{geometry_field}', record: {record:?}");
-            e
-        })?;
+        crate::wkt::wkt_reader::process_wkt_geom_n(&wkt, record_idx, processor).inspect_err(
+            |_e| {
+                // +2 to start at line 1 and to account for the header row
+                let line = record_idx + 2;
+                log::warn!("line {line}: invalid WKT: '{geometry_field}', record: {record:?}");
+            },
+        )?;
     }
 
     if !collection_started {
@@ -159,7 +159,7 @@ pub fn process_csv_features(
     let geometry_idx = headers
         .iter()
         .position(|f| f == geometry_column)
-        .ok_or(GeozeroError::ColumnNotFound)?;
+        .ok_or(CsvError::ColumnNotFound)?;
 
     for (feature_idx, record) in reader.into_records().enumerate() {
         let record = record?;
@@ -182,21 +182,17 @@ pub fn process_csv_features(
 
         processor.properties_end()?;
 
-        let geometry_field = record
-            .get(geometry_idx)
-            .ok_or(GeozeroError::ColumnNotFound)?;
+        let geometry_field = record.get(geometry_idx).ok_or(CsvError::ColumnNotFound)?;
 
         // Do all formats allow empty geometries?
         if !geometry_field.is_empty() {
             processor.geometry_begin()?;
-            crate::wkt::wkt_reader::read_wkt(&mut geometry_field.as_bytes(), processor).map_err(
-                |e| {
+            crate::wkt::wkt_reader::read_wkt(&mut geometry_field.as_bytes(), processor)
+                .inspect_err(|_e| {
                     // +2 to start at line 1 and to account for the header row
                     let line = feature_idx + 2;
                     log::warn!("line {line}: invalid WKT: '{geometry_field}', record: {record:?}");
-                    e
-                },
-            )?;
+                })?;
             processor.geometry_end()?;
         }
 

diff --git a/geozero/src/csv/mod.rs b/geozero/src/csv/mod.rs
@@ -1,7 +1,9 @@
 //! CSV conversions.
+mod csv_error;
 pub(crate) mod csv_reader;
 pub(crate) mod csv_writer;
 
+pub use csv_error::CsvError;
 pub use csv_reader::*;
 pub use csv_writer::*;
 

diff --git a/geozero/src/error.rs b/geozero/src/error.rs
@@ -39,6 +39,10 @@ pub enum GeozeroError {
     // General
     #[error("I/O error `{0}`")]
     IoError(#[from] std::io::Error),
+    // Format Specific
+    #[cfg(feature = "with-csv")]
+    #[error("CSV error `{0}`")]
+    CsvError(#[from] crate::csv::CsvError),
     #[cfg(feature = "with-mvt")]
     #[error("MVT error `{0}`")]
     MvtError(#[from] crate::mvt::MvtError),