Skip to content

Commit 94ae57a

Browse files
committed
Fix geo overlay tests
1 parent cce159a commit 94ae57a

File tree

4 files changed

+38
-6
lines changed

4 files changed

+38
-6
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/sedona-geo/src/to_geo.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,10 @@ pub fn item_to_geometry(geo: impl GeometryTrait<T = f64>) -> Result<Geometry> {
6969
}
7070

7171
// GeometryCollection causes issues because it has a recursive definition and won't work
72-
// with cargo run --release. Thus, we need our own version of this that limits the
73-
// recursion supported in a GeometryCollection.
72+
// with cargo run --release. Thus, we need our own version of this that works around this
73+
// problem by processing GeometryCollection using a free function instead of relying
74+
// on trait resolver.
75+
// See also https://github.com/geoarrow/geoarrow-rs/pull/956.
7476
fn to_geometry(item: impl GeometryTrait<T = f64>) -> Option<Geometry> {
7577
match item.as_type() {
7678
Point(geom) => geom.try_to_point().map(Geometry::Point),

rust/sedona-testing/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ criterion = { workspace = true, optional = true }
4242
datafusion-common = { workspace = true }
4343
datafusion-expr = { workspace = true }
4444
datafusion-physical-expr = { workspace = true }
45-
geo-traits = { workspace = true }
45+
geo-traits = { workspace = true, features = ["geo-types"] }
4646
geo-types = { workspace = true }
4747
parquet = { workspace = true, features = ["arrow", "snap", "zstd"] }
4848
rand = { workspace = true }
@@ -52,3 +52,4 @@ sedona-expr = { path = "../sedona-expr" }
5252
sedona-schema = { path = "../sedona-schema" }
5353
wkb = { workspace = true }
5454
wkt = { workspace = true }
55+
geo = { workspace = true }

rust/sedona-testing/src/compare.rs

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ use datafusion_common::{
2323
ScalarValue,
2424
};
2525
use datafusion_expr::ColumnarValue;
26+
use geo::Relate;
27+
use geo_traits::to_geo::ToGeoGeometry;
2628
use sedona_schema::datatypes::{SedonaType, WKB_GEOMETRY};
29+
use wkb::reader::Dimension;
2730

2831
use crate::create::create_scalar;
2932

@@ -104,7 +107,9 @@ pub fn assert_array_equal(actual: &ArrayRef, expected: &ArrayRef) {
104107
/// Panics if the values' are not equal, generating reasonable failure messages for geometry
105108
/// arrays where the default failure message would otherwise be uninformative.
106109
pub fn assert_scalar_equal_wkb_geometry(actual: &ScalarValue, expected_wkt: Option<&str>) {
107-
assert_scalar_equal(actual, &create_scalar(expected_wkt, &WKB_GEOMETRY));
110+
let expected = create_scalar(expected_wkt, &WKB_GEOMETRY);
111+
assert_eq!(actual.data_type(), DataType::Binary);
112+
assert_wkb_scalar_equal(actual, &expected);
108113
}
109114

110115
/// Assert two [`ScalarValue`]s are equal
@@ -205,9 +210,32 @@ fn assert_wkb_value_equal(
205210
)
206211
}
207212
(Some(actual_wkb), Some(expected_wkb)) => {
213+
// Quick test: if the binary of the WKB is the same, they are equal
208214
if actual_wkb != expected_wkb {
209-
let (actual_wkt, expected_wkt) = (format_wkb(actual_wkb), format_wkb(expected_wkb));
210-
panic!("{actual_label} != {expected_label}\n{actual_label}:\n {actual_wkt}\n{expected_label}:\n {expected_wkt}")
215+
// Binary of the WKB are not the same, but geometries could be topologically the same.
216+
let expected = wkb::reader::read_wkb(expected_wkb);
217+
let actual = wkb::reader::read_wkb(actual_wkb);
218+
let is_equals = match (expected, actual) {
219+
(Ok(expected_geom), Ok(actual_geom)) => {
220+
if expected_geom.dimension() == Dimension::Xy
221+
&& actual_geom.dimension() == Dimension::Xy
222+
{
223+
let expected_geom = expected_geom.to_geometry();
224+
let actual_geom = actual_geom.to_geometry();
225+
expected_geom.relate(&actual_geom).is_equal_topo()
226+
} else {
227+
// geo crate does not support 3D/4D geometry operations, so we fall back to using the result
228+
// of byte-wise comparison
229+
false
230+
}
231+
}
232+
_ => false,
233+
};
234+
if !is_equals {
235+
let (actual_wkt, expected_wkt) =
236+
(format_wkb(actual_wkb), format_wkb(expected_wkb));
237+
panic!("{actual_label} != {expected_label}\n{actual_label}:\n {actual_wkt}\n{expected_label}:\n {expected_wkt}")
238+
}
211239
}
212240
}
213241
}

0 commit comments

Comments
 (0)