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