paleolimbot commented on code in PR #165:
URL: https://github.com/apache/sedona-db/pull/165#discussion_r2392930925
##########
c/sedona-geos/Cargo.toml:
##########
@@ -31,6 +31,7 @@ criterion = { workspace = true }
sedona = { path = "../../rust/sedona" }
sedona-testing = { path = "../../rust/sedona-testing", features =
["criterion"] }
rstest = { workspace = true }
+geo = { workspace = true }
Review Comment:
Does this need `geo` or would `geo-types` suffice? (It seems odd to include
geo here?)
##########
rust/sedona-testing/src/compare.rs:
##########
@@ -205,9 +210,32 @@ fn assert_wkb_value_equal(
)
}
(Some(actual_wkb), Some(expected_wkb)) => {
+ // Quick test: if the binary of the WKB is the same, they are equal
if actual_wkb != expected_wkb {
- let (actual_wkt, expected_wkt) = (format_wkb(actual_wkb),
format_wkb(expected_wkb));
- panic!("{actual_label} != {expected_label}\n{actual_label}:\n
{actual_wkt}\n{expected_label}:\n {expected_wkt}")
+ // Binary of the WKB are not the same, but geometries could be
topologically the same.
+ let expected = wkb::reader::read_wkb(expected_wkb);
+ let actual = wkb::reader::read_wkb(actual_wkb);
Review Comment:
I would prefer to have this be a different function (in other words, I would
like to know when I am asserting topological equality vs.
coordiante-for-coordinate equality (within some tolerance) or byte-for-byte
equality).
##########
rust/sedona-testing/Cargo.toml:
##########
@@ -52,3 +52,4 @@ sedona-expr = { path = "../sedona-expr" }
sedona-schema = { path = "../sedona-schema" }
wkb = { workspace = true }
wkt = { workspace = true }
+geo = { workspace = true }
Review Comment:
I would prefer to keep `geo` out of sedona-testing, which is otherwise
algorithm library-neutral (individual tests can always use geo if they want!)
##########
Cargo.toml:
##########
@@ -22,6 +22,9 @@ members = [
"c/sedona-s2geography",
"c/sedona-tg",
"r/sedonadb/src/rust",
+ "rust/geo-traits-ext",
+ "rust/geo-generic-alg",
+ "rust/geo-test-fixtures",
Review Comment:
Should we rename these to `sedona-*`?
--
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]
