Copilot commented on code in PR #71:
URL: https://github.com/apache/sedona-db/pull/71#discussion_r2344275576
##########
rust/sedona-testing/src/datagen.rs:
##########
@@ -756,15 +758,20 @@ fn generate_non_overlapping_sub_rectangles(num_parts:
usize, bounds: &Rect) -> V
tiles
}
-fn generate_circular_vertices(
+fn generate_circular_vertices<R: rand::Rng>(
+ rng: &mut R,
center_x: f64,
center_y: f64,
radius: f64,
num_vertices: usize,
closed: bool,
) -> Vec<Coord> {
let mut out = Vec::new();
- let mut angle: f64 = 0.0;
+
+ // Randomize starting angle (0 to 2 * PI)
+ let start_angle_dist = Uniform::new(0.0, 2.0 * PI);
Review Comment:
The constant `PI` is not imported or defined in this scope. You need to add
`use std::f64::consts::PI;` to the imports at the top of the file.
##########
python/sedonadb/tests/test_sjoin.py:
##########
@@ -89,52 +90,53 @@ def test_spatial_join(join_type, on):
],
)
def test_spatial_join_geography(join_type, on):
- eng_sedonadb = SedonaDB.create_or_skip()
- eng_postgis = PostGIS.create_or_skip()
-
- # Select two sets of bounding boxes that cross the antimeridian,
- # which would be disjoint on a Euclidean plane. A geography join will
produce non-empty results,
- # whereas a geometry join would not.
- west_most_bound = [-190, -10, -170, 10]
- east_most_bound = [170, -10, 190, 10]
- options = json.dumps(
- {
- "geom_type": "Point",
- "num_parts_range": [2, 10],
- "vertices_per_linestring_range": [2, 10],
- "bounds": west_most_bound,
- "size_range": [0.1, 5],
- "seed": 42,
- }
- )
- df_point = eng_sedonadb.execute_and_collect(
- f"SELECT id, ST_SetSRID(ST_GeogFromWKB(ST_AsBinary(geometry)), 4326)
geog, dist FROM sd_random_geometry('{options}') LIMIT 100"
- )
- options = json.dumps(
- {
- "geom_type": "Polygon",
- "polygon_hole_rate": 0.5,
- "num_parts_range": [2, 10],
- "vertices_per_linestring_range": [2, 10],
- "bounds": east_most_bound,
- "size_range": [0.1, 5],
- "seed": 43,
- }
- )
- df_polygon = eng_sedonadb.execute_and_collect(
- f"SELECT id, ST_SetSRID(ST_GeogFromWKB(ST_AsBinary(geometry)), 4326)
geog, dist FROM sd_random_geometry('{options}') LIMIT 100"
- )
- eng_sedonadb.create_table_arrow("sjoin_geog1", df_point)
- eng_sedonadb.create_table_arrow("sjoin_geog2", df_polygon)
- eng_postgis.create_table_arrow("sjoin_geog1", df_point)
- eng_postgis.create_table_arrow("sjoin_geog2", df_polygon)
+ with (
+ SedonaDB.create_or_skip() as eng_sedonadb,
+ PostGIS.create_or_skip() as eng_postgis,
+ ):
+ # Select two sets of bounding boxes that cross the antimeridian,
+ # which would be disjoint on a Euclidean plane. A geography join will
produce non-empty results,
+ # whereas a geometry join would not.
+ west_most_bound = [-190, -10, -170, 10]
+ east_most_bound = [170, -10, 190, 10]
+ options = json.dumps(
+ {
+ "geom_type": "Point",
+ "num_parts_range": [2, 10],
+ "vertices_per_linestring_range": [2, 10],
+ "bounds": west_most_bound,
+ "size_range": [0.1, 5],
+ "seed": 43,
Review Comment:
[nitpick] The seed value changed from 42 to 43 in the second geometry
options. While this change creates different data, it's not clear why this
specific seed was chosen. Consider adding a comment explaining the rationale
for using different seeds.
--
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]
