georust / geozero

Zero-Copy reading and writing of geospatial data.
Apache License 2.0
322 stars 30 forks source link

Error refactoring #138

Open nyurik opened 1 year ago

nyurik commented 1 year ago

GeoZero has one Error enum, and many of the values are String objects that contain the actual error information, sometimes formatted.

I think it would better to refactor it so that each specific error can be easily analyzed without any extra performance cost.

Proposed structure

enum GeozeroError {
    arrow: ArrowError, csv: CsvError, gdal: GdalError, geojson: GeojsonError, geos: GeosError, gpkg: GpkgError, gpx: GpxError, mvt: MvtError, postgis: PostgisError, svg: SvgError, tessellator: TessellatorError, wkb: WkbError, wkt: WktError,
}

Each one of the sub-errors are their own enums with the actual values. So instead of this in src/mvt/mvt_reader.rs:

return Err(GeozeroError::Feature(format!(
    "invalid feature.tags length: {:?}", feature.tags.len()
)));

we will have this:

return Err(MvtError::InvalidFeatureTagsLength(feature.tags.len());

Note that thiserror has #[from] to simplify MvtError -> GeozeroError conversion.

Existing errors

Here are all the error usages at the moment (some of these are used multiple times)

GeozeroError::ColumnNotFound
GeozeroError::ColumnType(stringify!($e), format!("{v:?}")
GeozeroError::Dataset(error
GeozeroError::Feature("invalid feature.tags length: {feature_tags_count:?}"
GeozeroError::Feature("invalid key index {key_idx}"
GeozeroError::Feature("invalid value index {value_idx}"
GeozeroError::Geometry("CoordSeq missing"
GeozeroError::Geometry("Invalid UTF-8 encoding"
GeozeroError::Geometry("Missing Geometry"
GeozeroError::Geometry("Missing LineStrings for Polygon"
GeozeroError::Geometry("Missing container for LineString"
GeozeroError::Geometry("Missing container for Polygon"
GeozeroError::Geometry("Missing polygons for MultiPolygon"
GeozeroError::Geometry("No LineStrings for MultiLineString"
GeozeroError::Geometry("No coords for LineString"
GeozeroError::Geometry("No coords for MultiPoint"
GeozeroError::Geometry("No coords for Point"
GeozeroError::Geometry("Not ready for coords"
GeozeroError::Geometry("The input was an empty Point, but the output doesn't support empty Points"
GeozeroError::Geometry("Too few coordinates in line or ring"
GeozeroError::Geometry("Unexpected geometry type"
GeozeroError::Geometry("test"
GeozeroError::Geometry(error
GeozeroError::Geometry(format!("Unsupported geometry type {geometry_type}"
GeozeroError::GeometryFormat
GeozeroError::IoError(io_err
GeozeroError::Property(format!("unsupported value type for key {key}"
nrhill1 commented 1 month ago

I’m thinking of taking this task on…is it still open? Or was it fixed in a different issue?

michaelkirk commented 1 month ago

Take a look at the code and see?