BlakeOrth commented on code in PR #8222:
URL: https://github.com/apache/arrow-rs/pull/8222#discussion_r2299035292


##########
arrow-schema/src/extension/canonical/geometry.rs:
##########
@@ -0,0 +1,136 @@
+use crate::extension::ExtensionType;
+use crate::ArrowError;
+
+/// Geospatial features in the WKB format with linear/planar edges 
interpolation
+#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
+pub struct Geometry {
+    crs: Option<String>,

Review Comment:
   Based on the verbage in the spec I'm wondering if it would be better to have 
`crs` be required instead of optional. 
   
   > The default CRS `OGC:CRS84` means that the geospatial features must be 
stored
   in the order of longitude/latitude based on the WGS84 datum.
   > 
   > Custom CRS can be specified by a string value. It is recommended to use an
   identifier-based approach like srid.
   
   My reading of this is that CRS is actually required for all Geometry and 
Geography types, and users may supply a custom string, but if not it will 
always be `String::from("OGC:CRS84")`. I suspect having this be required may 
also simplify some of the downstream code because there will be less `Option` 
handling that needs to happen.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to