Copilot commented on code in PR #60:
URL:
https://github.com/apache/sedona-spatialbench/pull/60#discussion_r2508555416
##########
spatialbench/tests/geometry_tests.rs:
##########
@@ -0,0 +1,136 @@
+//! Geometry validation tests for generated spatial data.
+//!
+//! This test suite validates that all generated geometries meet quality and
correctness standards:
+//! - **Coordinate validity**: Longitude [-180°, 180°], latitude [-90°, 90°]
+//! - **OGC compliance**: Valid topology, counter-clockwise winding for
exterior rings
+//! - **Precision constraints**: Coordinates limited to 9 decimal places
+//! - **Antimeridian handling**: No dateline crossings in generated polygons
+//!
+//! These tests ensure spatial data quality and compatibility with spatial
databases and GIS tools.
+
+#[cfg(test)]
+mod geometry_tests {
+ use geo::{Validation, Winding};
+ use spatialbench::generators::{BuildingGenerator, TripGenerator};
+ use spatialbench::spatial::geometry::GEOMETRY_PRECISION;
+ use spatialbench::spatial::utils::crosses_dateline;
+
+ #[test]
+ fn test_trip_coordinates_are_valid() {
+ let generator = TripGenerator::new(0.1, 1, 1);
+ let trips: Vec<_> = generator.iter().collect();
+
+ for trip in trips {
+ // Check pickup coordinates are valid
+ assert!(
+ trip.t_pickuploc.x() >= -180.0 && trip.t_pickuploc.x() <=
180.0,
+ "Pickup longitude out of range: {}",
+ trip.t_pickuploc.x()
+ );
+ assert!(
+ trip.t_pickuploc.y() >= -90.0 && trip.t_pickuploc.y() <= 90.0,
+ "Pickup latitude out of range: {}",
+ trip.t_pickuploc.y()
+ );
+
+ // Check dropoff coordinates are valid
+ assert!(
+ trip.t_dropoffloc.x() >= -180.0 && trip.t_dropoffloc.x() <=
180.0,
+ "Dropoff longitude out of range: {}",
+ trip.t_dropoffloc.x()
+ );
+ assert!(
+ trip.t_dropoffloc.y() >= -90.0 && trip.t_dropoffloc.y() <=
90.0,
+ "Dropoff latitude out of range: {}",
+ trip.t_dropoffloc.y()
+ );
+
+ // Check coordinates have proper precision (9 decimal places)
+ let pickup_x_precision =
+ (trip.t_pickuploc.x() * GEOMETRY_PRECISION).round() /
GEOMETRY_PRECISION;
+ assert_eq!(trip.t_pickuploc.x(), pickup_x_precision);
+
+ let dropoff_x_precision =
+ (trip.t_dropoffloc.x() * GEOMETRY_PRECISION).round() /
GEOMETRY_PRECISION;
+ assert_eq!(trip.t_dropoffloc.x(), dropoff_x_precision);
+ }
+ }
+
+ #[test]
+ fn test_building_polygons_are_valid() {
+ let generator = BuildingGenerator::new(10.0, 1, 1);
+ let buildings: Vec<_> = generator.iter().collect();
+
+ for building in buildings {
+ let polygon = &building.b_boundary;
+
+ assert!(
+ polygon.is_valid(),
+ "Building {} has invalid polygon",
+ building.b_buildingkey
+ );
+ }
+ }
+
+ #[test]
+ fn test_building_polygon_winding() {
+ // Create a generator with a small scale factor
+ let generator = BuildingGenerator::new(1.0, 1, 1);
+ let buildings: Vec<_> = generator.iter().collect();
+
+ // Check that all building polygons have counter-clockwise winding
+ for building in buildings.iter() {
+ let exterior = building.b_boundary.exterior();
+ assert!(
+ exterior.is_ccw(),
+ "Building {} polygon should have counter-clockwise winding",
+ building.b_buildingkey
+ );
+ }
+ }
+
+ #[test]
+ fn test_coordinate_precision() {
+ let generator = TripGenerator::new(0.01, 1, 1);
+ let trips: Vec<_> = generator.iter().collect();
+
+ for trip in trips {
+ // Check 8 decimal place precision
+ let pickup_x_str = format!("{:.9}", trip.t_pickuploc.x());
+ let pickup_y_str = format!("{:.9}", trip.t_pickuploc.y());
+ let dropoff_x_str = format!("{:.9}", trip.t_dropoffloc.x());
+ let dropoff_y_str = format!("{:.9}", trip.t_dropoffloc.y());
+
+ // Verify no extra precision beyond 8 decimals
Review Comment:
Comment mentions '8 decimals' but the validation is for 9 decimal places
based on the format string and GEOMETRY_PRECISION constant.
```suggestion
// Check 9 decimal place precision
let pickup_x_str = format!("{:.9}", trip.t_pickuploc.x());
let pickup_y_str = format!("{:.9}", trip.t_pickuploc.y());
let dropoff_x_str = format!("{:.9}", trip.t_dropoffloc.x());
let dropoff_y_str = format!("{:.9}", trip.t_dropoffloc.y());
// Verify no extra precision beyond 9 decimals
```
##########
spatialbench/src/spatial/utils/antimeridian.rs:
##########
@@ -0,0 +1,258 @@
+use geo::{Centroid, LineString, Polygon};
+
+/// Wraps a longitude value to ensure it stays within the valid range of
[-180, 180] degrees.
+///
+/// Longitude is a circular coordinate:
+/// - If longitude exceeds 180°, it wraps around from the eastern hemisphere
back to the western hemisphere.
+/// - If longitude is below -180°, it wraps around from the western hemisphere
back to the eastern hemisphere.
+pub fn wrap_around_longitude(mut lon: f64) -> f64 {
+ while lon > 180.0 {
+ lon -= 360.0;
+ }
+ while lon < -180.0 {
+ lon += 360.0;
+ }
+ lon
+}
+
+/// Checks if a polygon crosses the dateline (antimeridian at ±180°)
+pub fn crosses_dateline(polygon: &Polygon) -> bool {
+ let coords = polygon.exterior().coords();
+ let mut has_east = false;
+ let mut has_west = false;
+
+ for coord in coords {
+ if (coord.x > 90.0 && coord.x <= 180.0) || coord.x < -180.0 {
Review Comment:
Condition `coord.x < -180.0` should not be possible if coordinates are
properly wrapped to [-180, 180] range. This suggests either the function
expects unwrapped input or there's a logical issue with the dateline crossing
detection.
```suggestion
if coord.x > 90.0 && coord.x <= 180.0 {
```
##########
spatialbench/src/spatial/utils/antimeridian.rs:
##########
@@ -0,0 +1,258 @@
+use geo::{Centroid, LineString, Polygon};
+
+/// Wraps a longitude value to ensure it stays within the valid range of
[-180, 180] degrees.
+///
+/// Longitude is a circular coordinate:
+/// - If longitude exceeds 180°, it wraps around from the eastern hemisphere
back to the western hemisphere.
+/// - If longitude is below -180°, it wraps around from the western hemisphere
back to the eastern hemisphere.
+pub fn wrap_around_longitude(mut lon: f64) -> f64 {
+ while lon > 180.0 {
+ lon -= 360.0;
+ }
+ while lon < -180.0 {
+ lon += 360.0;
+ }
+ lon
+}
+
+/// Checks if a polygon crosses the dateline (antimeridian at ±180°)
+pub fn crosses_dateline(polygon: &Polygon) -> bool {
+ let coords = polygon.exterior().coords();
+ let mut has_east = false;
+ let mut has_west = false;
+
+ for coord in coords {
+ if (coord.x > 90.0 && coord.x <= 180.0) || coord.x < -180.0 {
+ has_east = true;
+ }
+ if coord.x > 180.0 || (coord.x >= -180.0 && coord.x < -90.0) {
+ has_west = true;
+ }
+ if has_east && has_west {
Review Comment:
Condition `coord.x > 180.0` should not be possible if coordinates are
properly wrapped to [-180, 180] range. The logic for detecting dateline
crossing appears inconsistent with the function's documented purpose.
```suggestion
let coords: Vec<_> = polygon.exterior().coords().collect();
if coords.len() < 2 {
return false;
}
for i in 0..coords.len() - 1 {
let lon1 = coords[i].x;
let lon2 = coords[i + 1].x;
if (lon1 - lon2).abs() > 180.0 {
```
##########
spatialbench/tests/geometry_tests.rs:
##########
@@ -0,0 +1,136 @@
+//! Geometry validation tests for generated spatial data.
+//!
+//! This test suite validates that all generated geometries meet quality and
correctness standards:
+//! - **Coordinate validity**: Longitude [-180°, 180°], latitude [-90°, 90°]
+//! - **OGC compliance**: Valid topology, counter-clockwise winding for
exterior rings
+//! - **Precision constraints**: Coordinates limited to 9 decimal places
+//! - **Antimeridian handling**: No dateline crossings in generated polygons
+//!
+//! These tests ensure spatial data quality and compatibility with spatial
databases and GIS tools.
+
+#[cfg(test)]
+mod geometry_tests {
+ use geo::{Validation, Winding};
+ use spatialbench::generators::{BuildingGenerator, TripGenerator};
+ use spatialbench::spatial::geometry::GEOMETRY_PRECISION;
+ use spatialbench::spatial::utils::crosses_dateline;
+
+ #[test]
+ fn test_trip_coordinates_are_valid() {
+ let generator = TripGenerator::new(0.1, 1, 1);
+ let trips: Vec<_> = generator.iter().collect();
+
+ for trip in trips {
+ // Check pickup coordinates are valid
+ assert!(
+ trip.t_pickuploc.x() >= -180.0 && trip.t_pickuploc.x() <=
180.0,
+ "Pickup longitude out of range: {}",
+ trip.t_pickuploc.x()
+ );
+ assert!(
+ trip.t_pickuploc.y() >= -90.0 && trip.t_pickuploc.y() <= 90.0,
+ "Pickup latitude out of range: {}",
+ trip.t_pickuploc.y()
+ );
+
+ // Check dropoff coordinates are valid
+ assert!(
+ trip.t_dropoffloc.x() >= -180.0 && trip.t_dropoffloc.x() <=
180.0,
+ "Dropoff longitude out of range: {}",
+ trip.t_dropoffloc.x()
+ );
+ assert!(
+ trip.t_dropoffloc.y() >= -90.0 && trip.t_dropoffloc.y() <=
90.0,
+ "Dropoff latitude out of range: {}",
+ trip.t_dropoffloc.y()
+ );
+
+ // Check coordinates have proper precision (9 decimal places)
+ let pickup_x_precision =
+ (trip.t_pickuploc.x() * GEOMETRY_PRECISION).round() /
GEOMETRY_PRECISION;
+ assert_eq!(trip.t_pickuploc.x(), pickup_x_precision);
+
+ let dropoff_x_precision =
+ (trip.t_dropoffloc.x() * GEOMETRY_PRECISION).round() /
GEOMETRY_PRECISION;
+ assert_eq!(trip.t_dropoffloc.x(), dropoff_x_precision);
+ }
+ }
+
+ #[test]
+ fn test_building_polygons_are_valid() {
+ let generator = BuildingGenerator::new(10.0, 1, 1);
+ let buildings: Vec<_> = generator.iter().collect();
+
+ for building in buildings {
+ let polygon = &building.b_boundary;
+
+ assert!(
+ polygon.is_valid(),
+ "Building {} has invalid polygon",
+ building.b_buildingkey
+ );
+ }
+ }
+
+ #[test]
+ fn test_building_polygon_winding() {
+ // Create a generator with a small scale factor
+ let generator = BuildingGenerator::new(1.0, 1, 1);
+ let buildings: Vec<_> = generator.iter().collect();
+
+ // Check that all building polygons have counter-clockwise winding
+ for building in buildings.iter() {
+ let exterior = building.b_boundary.exterior();
+ assert!(
+ exterior.is_ccw(),
+ "Building {} polygon should have counter-clockwise winding",
+ building.b_buildingkey
+ );
+ }
+ }
+
+ #[test]
+ fn test_coordinate_precision() {
+ let generator = TripGenerator::new(0.01, 1, 1);
+ let trips: Vec<_> = generator.iter().collect();
+
+ for trip in trips {
+ // Check 8 decimal place precision
Review Comment:
Comment mentions '8 decimal place precision' but the actual precision being
validated is 9 decimal places (format!(\"{:.9}\") and GEOMETRY_PRECISION =
1_000_000_000.0).
```suggestion
// Check 9 decimal place precision
```
##########
spatialbench/src/spatial/utils/antimeridian.rs:
##########
@@ -0,0 +1,258 @@
+use geo::{Centroid, LineString, Polygon};
+
+/// Wraps a longitude value to ensure it stays within the valid range of
[-180, 180] degrees.
+///
+/// Longitude is a circular coordinate:
+/// - If longitude exceeds 180°, it wraps around from the eastern hemisphere
back to the western hemisphere.
+/// - If longitude is below -180°, it wraps around from the western hemisphere
back to the eastern hemisphere.
+pub fn wrap_around_longitude(mut lon: f64) -> f64 {
+ while lon > 180.0 {
+ lon -= 360.0;
+ }
+ while lon < -180.0 {
+ lon += 360.0;
+ }
+ lon
+}
+
+/// Checks if a polygon crosses the dateline (antimeridian at ±180°)
+pub fn crosses_dateline(polygon: &Polygon) -> bool {
+ let coords = polygon.exterior().coords();
+ let mut has_east = false;
+ let mut has_west = false;
+
+ for coord in coords {
+ if (coord.x > 90.0 && coord.x <= 180.0) || coord.x < -180.0 {
+ has_east = true;
+ }
+ if coord.x > 180.0 || (coord.x >= -180.0 && coord.x < -90.0) {
+ has_west = true;
+ }
+ if has_east && has_west {
+ return true;
+ }
+ }
+ false
+}
+
+/// Clamps a polygon's longitude coordinates to one side of the antimeridian
(±180°).
+///
+/// When a polygon crosses the dateline, this function constrains its
coordinates to remain
+/// on either the eastern (0° to 180°) or western (-180° to 0°) hemisphere
based on where
+/// the polygon's centroid is located.
+///
+/// # Behavior
+/// - If the centroid is in the eastern hemisphere (≥ 0°), coordinates are
clamped to [0°, 180°]
+/// or kept at their original values if already within [-180°, 0°]
+/// - If the centroid is in the western hemisphere (< 0°), coordinates are
clamped to [-180°, 0°]
+/// or kept at their original values if already within [0°, 180°]
+/// - Latitude values (y-coordinates) remain unchanged
+///
+pub fn clamp_polygon_to_dateline(polygon: &Polygon) -> Polygon {
+ let centroid = polygon.centroid().expect("Polygon should have centroid");
+ let east_bound = centroid.x() >= 0.0;
+ let keep_east = (centroid.x() >= 0.0 && centroid.x() <= 180.0) ||
(centroid.x() < -180.0);
Review Comment:
Condition `centroid.x() < -180.0` should never be true if coordinates are
normalized to [-180, 180] range. This suggests the function may not handle all
edge cases correctly.
```suggestion
let centroid_lon = wrap_around_longitude(centroid.x());
let east_bound = centroid_lon >= 0.0;
let keep_east = centroid_lon >= 0.0 && centroid_lon <= 180.0;
```
--
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]
