paleolimbot commented on code in PR #713: URL: https://github.com/apache/arrow-go/pull/713#discussion_r2956368457
########## arrow/extensions/geoarrow/point.go: ########## @@ -0,0 +1,355 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package geoarrow + +import ( + "encoding/json" + "fmt" + "reflect" + "strconv" + "strings" + + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + "github.com/apache/arrow-go/v18/arrow/memory" + arrowjson "github.com/apache/arrow-go/v18/internal/json" +) + +type PointType struct { + arrow.ExtensionBase + Extension +} + +type PointValue struct { + coords []float64 + dim CoordinateDimension +} + +func NewPointValue(x, y float64) PointValue { + return PointValue{coords: []float64{x, y}, dim: XY} +} + +func NewPointValueZ(x, y, z float64) PointValue { + return PointValue{coords: []float64{x, y, z}, dim: XYZ} +} + +func NewPointValueM(x, y, m float64) PointValue { + return PointValue{coords: []float64{x, y, m}, dim: XYM} +} + +func NewPointValueZM(x, y, z, m float64) PointValue { + return PointValue{coords: []float64{x, y, z, m}, dim: XYZM} +} + +func (v PointValue) X() float64 { + return v.coords[0] +} + +func (v PointValue) Y() float64 { + return v.coords[1] +} + +func (v PointValue) Z() float64 { + if v.dim != XYZ && v.dim != XYZM { + return 0 + } + return v.coords[2] Review Comment: I personally like returning `NaN` here but I know that the default value of 0 is more common (I just like having this be explicit). ########## arrow/extensions/geoarrow/geometry.go: ########## @@ -0,0 +1,92 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package geoarrow + +import ( + "encoding/json" + "fmt" +) + +type CoordinateDimension int + +const ( + XY CoordinateDimension = iota + XYZ + XYM + XYZM +) + +func (d CoordinateDimension) NDim() int { + switch d { + case XY: + return 2 + case XYZ, XYM: + return 3 + case XYZM: + return 4 + default: + return 0 + } +} + +type Layout int + +const ( + Separate Layout = iota + Interleaved +) + +// GeometryTypeID represents the specific geometry type (e.g., Point, +// LineString, Polygon) and its coordinate dimension (e.g., XY, XYZ, XYM, XYZM) +type GeometryTypeID int + +// As defined in https://github.com/geoarrow/geoarrow +const ( + PointID GeometryTypeID = 1 + LineStringID GeometryTypeID = 2 + PolygonID GeometryTypeID = 3 + MultiPointID GeometryTypeID = 4 + MultiLineStringID GeometryTypeID = 5 + MultiPolygonID GeometryTypeID = 6 Review Comment: May be worth repeating here that `value % 10` and `value / 10` have meaning here (can be used to separate geometry type and dimensions) ########## arrow/extensions/geoarrow/extension.go: ########## @@ -0,0 +1,101 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package geoarrow + +import ( + "encoding/json" + + "github.com/apache/arrow-go/v18/arrow" + "github.com/apache/arrow-go/v18/arrow/array" + arrowjson "github.com/apache/arrow-go/v18/internal/json" +) + +const ( + ExtensionNameWKB = "geoarrow.wkb" + ExtensionNamePoint = "geoarrow.point" + // TODO ExtensionNamePolygon = "geoarrow.polygon" + // TODO ExtensionNameWKT = "geoarrow.wkt" + // TODO ExtensionNameLineString = "geoarrow.linestring" + // TODO ExtensionNameMultiPoint = "geoarrow.multipoint" + // TODO ExtensionNameMultiLineString = "geoarrow.multilinestring" + // TODO ExtensionNameMultiPolygon = "geoarrow.multipolygon" + // TODO ExtensionNameGeometry = "geoarrow.geometry" + // TODO ExtensionNameGeometryCollection = "geoarrow.geometrycollection" + // TODO ExtensionNameBox = "geoarrow.box" +) + +// GeometryType represents a GeoArrow geometry type with arbitrary +// format and encoding. +type GeometryType[V GeometryValue] interface { + arrow.ExtensionType + valueFromArray(a array.ExtensionArray, i int) V + appendValueToBuilder(b array.Builder, v V) + valueFromString(s string) (V, error) + unmarshalJSONOne(dec *arrowjson.Decoder) (V, bool, error) +} + +// Extension is a base struct that can be embedded in a GeoArrow extension +// +// type to provide common metadata and serialization logic. +type Extension struct { + meta Metadata +} + +// Metadata returns the GeoArrow metadata associated with this extension type. +func (e *Extension) Metadata() Metadata { + return e.meta +} + +// Serialize returns the JSON string representation of the GeoArrow metadata for +// this extension type. +func (e *Extension) Serialize() string { + if e == nil { + return "" + } + + // Ignore errors since Metadata fields are well-defined and should always + // serialize successfully + serialized, _ := json.Marshal(e.meta) + return string(serialized) +} + +// Equal compares two Extension instances for equality based on their metadata. +func (e *Extension) Equal(other *Extension) bool { + if e == nil && other == nil { + return true + } + if e == nil || other == nil { + return false + } + if e.meta.CRSType != other.meta.CRSType { + return false + } + if e.meta.Edges != other.meta.Edges { + return false + } + + if len(e.meta.CRS) != len(other.meta.CRS) { + return false + } + for i, r := range e.meta.CRS { + if r != other.meta.CRS[i] { + return false + } + } Review Comment: I am not sure if you want to handle CRS equality properly here (we handle it here: https://github.com/apache/sedona-db/blob/4fcf9edf81fe5010a3b51475c243f155942e5541/rust/sedona-schema/src/crs.rs#L290-L300 ). -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
