Merge #349

349: Handling cases where feature may not have any geometry at all r=lnicola a=phayes

- [✓] I agree to follow the project's [code of conduct](https://github.com/georust/gdal/blob/master/CODE_OF_CONDUCT.md).
- [✓] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

I've run into a few instances where I'm loading data uploaded from users (for example CSV files) where GDAL isn't able to find any geometry.  In instances where you might be working with user-provided data with no geometry, calls to `Feature::geometry()` simply panic. 

This PR documents renders `Feature::geometry` non-panicking by returning an `Option<&Geometry>`

Co-authored-by: Patrick Hayes <[email protected]>

CHANGES.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+- **Breaking**: `Feature::geometry` returns an `Option<&Geometry>` instead of `&Geometry`. Calls to `Feature::geometry` will no longer panic.
+
+  - <https://github.com/georust/gdal/pull/349>
+
 - **Breaking**: `RasterBand::band_type` returns the `GdalDataType` enum instead of `GDALDataType::Type` ordinal. Fixes [#333](https://github.com/georust/gdal/issues/333)
 
   - <https://github.com/georust/gdal/pull/334>

examples/read_write_ogr.rs

@@ -39,13 +39,18 @@ fn run() -> Result<()> {
     let defn = Defn::from_layer(&lyr);
 
     for feature_a in layer_a.features() {
-        // Get the original geometry:
-        let geom = feature_a.geometry();
-        // Get a new transformed geometry:
-        let new_geom = geom.transform(&htransform)?;
-        // Create the new feature, set its geometry:
+        // Create the new feature
         let mut ft = Feature::new(&defn)?;
-        ft.set_geometry(new_geom)?;
+
+        // Get the original geometry
+        if let Some(geom) = feature_a.geometry() {
+            // Get a new transformed geometry:
+            let new_geom = geom.transform(&htransform)?;
+
+            // Set the new feature's geometry
+            ft.set_geometry(new_geom)?;
+        }
+
         // copy each field value of the feature:
         for fd in &fields_defn {
             if let Some(value) = feature_a.field(&fd.0)? {

examples/read_write_ogr_datetime.rs

@@ -31,7 +31,9 @@ fn run() -> gdal::errors::Result<()> {
 
     for feature_a in layer_a.features() {
         let mut ft = Feature::new(&defn)?;
-        ft.set_geometry(feature_a.geometry().clone())?;
+        if let Some(geom) = feature_a.geometry() {
+            ft.set_geometry(geom.clone())?;
+        }
         // copy each field value of the feature:
         for field in defn.fields() {
             ft.set_field(

src/vector/feature.rs

@@ -449,13 +449,18 @@ impl<'a> Feature<'a> {
         }
     }
 
-    /// Get the field's geometry.
-    pub fn geometry(&self) -> &Geometry {
-        if !self.geometry[0].has_gdal_ptr() {
-            let c_geom = unsafe { gdal_sys::OGR_F_GetGeometryRef(self.c_feature) };
-            unsafe { self.geometry[0].set_c_geometry(c_geom) };
+    /// Get the feature's geometry.
+    pub fn geometry(&self) -> Option<&Geometry> {
+        match self.geometry.first() {
+            Some(geom) => {
+                if !geom.has_gdal_ptr() {
+                    let c_geom = unsafe { gdal_sys::OGR_F_GetGeometryRef(self.c_feature) };
+                    unsafe { geom.set_c_geometry(c_geom) };
+                }
+                Some(geom)
+            }
+            None => None,
         }
-        &self.geometry[0]
     }
 
     pub fn geometry_by_name(&self, field_name: &str) -> Result<&Geometry> {
@@ -476,7 +481,7 @@ impl<'a> Feature<'a> {
         if idx >= self.geometry.len() {
             return Err(GdalError::InvalidFieldIndex {
                 index: idx,
-                method_name: "geometry_by_name",
+                method_name: "geometry_by_index",
             });
         }
         if !self.geometry[idx].has_gdal_ptr() {

src/vector/mod.rs

@@ -26,7 +26,7 @@
 //!         // need random access.
 //!         let fid = feature.fid().unwrap_or(0);
 //!         // Summarize the geometry
-//!         let geometry = feature.geometry();
+//!         let geometry = feature.geometry().unwrap();
 //!         let geom_type = geometry_type_to_name(geometry.geometry_type());
 //!         let geom_len = geometry.get_point_vec().len();
 //!         println!("    Feature fid={fid:?}, geometry_type='{geom_type}', geometry_len={geom_len}");

src/vector/vector_tests/mod.rs

@@ -523,7 +523,7 @@ mod tests {
     #[test]
     fn test_geom_accessors() {
         with_feature("roads.geojson", 236194095, |feature| {
-            let geom = feature.geometry();
+            let geom = feature.geometry().unwrap();
             assert_eq!(geom.geometry_type(), OGRwkbGeometryType::wkbLineString);
             let coords = geom.get_point_vec();
             assert_eq!(
@@ -587,7 +587,7 @@ mod tests {
     #[test]
     fn test_wkt() {
         with_feature("roads.geojson", 236194095, |feature| {
-            let wkt = feature.geometry().wkt().unwrap();
+            let wkt = feature.geometry().unwrap().wkt().unwrap();
             let wkt_ok = format!(
                 "{}{}",
                 "LINESTRING (26.1019276 44.4302748,",
@@ -600,7 +600,7 @@ mod tests {
     #[test]
     fn test_json() {
         with_feature("roads.geojson", 236194095, |feature| {
-            let json = feature.geometry().json();
+            let json = feature.geometry().unwrap().json();
             let json_ok = format!(
                 "{}{}{}{}",
                 "{ \"type\": \"LineString\", \"coordinates\": [ ",
@@ -778,7 +778,7 @@ mod tests {
             let ds = Dataset::open(fixture("output.geojson")).unwrap();
             let mut layer = ds.layer(0).unwrap();
             let ft = layer.features().next().unwrap();
-            assert_eq!(ft.geometry().wkt().unwrap(), "POINT (1 2)");
+            assert_eq!(ft.geometry().unwrap().wkt().unwrap(), "POINT (1 2)");
             assert_eq!(
                 ft.field("Name").unwrap().unwrap().into_string(),
                 Some("Feature 1".to_string())