kylebarron commented on code in PR #8222:
URL: https://github.com/apache/arrow-rs/pull/8222#discussion_r2298959417
##########
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>,
+}
+
+impl Geometry {
+ /// Create a new Geometry extension type with an optional CRS.
+ pub fn new(crs: Option<String>) -> Self {
+ Self { crs }
+ }
+
+ /// Get the CRS of the Geometry type, if any.
+ pub fn crs(&self) -> Option<&str> {
+ self.crs.as_deref()
+ }
+}
+
+impl ExtensionType for Geometry {
+ const NAME: &'static str = "geoarrow.wkb";
+
+ type Metadata = ();
Review Comment:
Should this type be `String`? In `geoarrow-schema` I have a separate
`Metadata` struct defined, and store this type as `Arc<Metadata>`:
https://docs.rs/geoarrow-schema/latest/geoarrow_schema/struct.WkbType.html#associatedtype.Metadata
In this case we might have different metadata for the Geometry/Geography
types.
##########
parquet/src/basic.rs:
##########
@@ -894,8 +906,10 @@ impl From<LogicalType> for parquet::LogicalType {
LogicalType::Uuid =>
parquet::LogicalType::UUID(Default::default()),
LogicalType::Float16 =>
parquet::LogicalType::FLOAT16(Default::default()),
LogicalType::Variant =>
parquet::LogicalType::VARIANT(Default::default()),
- LogicalType::Geometry =>
parquet::LogicalType::GEOMETRY(Default::default()),
- LogicalType::Geography =>
parquet::LogicalType::GEOGRAPHY(Default::default()),
+ LogicalType::Geometry { crs } =>
parquet::LogicalType::GEOMETRY(GeometryType { crs }),
+ LogicalType::Geography { crs, algorithm } => {
+ parquet::LogicalType::GEOGRAPHY(GeographyType { crs, algorithm
})
+ }
Review Comment:
And likewise for converting back from GeoArrow CRS to parquet logical type
CRS
##########
parquet/src/basic.rs:
##########
@@ -850,8 +859,11 @@ impl From<parquet::LogicalType> for LogicalType {
parquet::LogicalType::UUID(_) => LogicalType::Uuid,
parquet::LogicalType::FLOAT16(_) => LogicalType::Float16,
parquet::LogicalType::VARIANT(_) => LogicalType::Variant,
- parquet::LogicalType::GEOMETRY(_) => LogicalType::Geometry,
- parquet::LogicalType::GEOGRAPHY(_) => LogicalType::Geography,
+ parquet::LogicalType::GEOMETRY(t) => LogicalType::Geometry { crs:
t.crs },
+ parquet::LogicalType::GEOGRAPHY(t) => LogicalType::Geography {
+ crs: t.crs,
+ algorithm: t.algorithm,
+ },
Review Comment:
This needs to be fixed to do a small CRS transform here, since Parquet
defines a string: `projjson:<crs body>`.
--
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
