Copilot commented on code in PR #70:
URL: https://github.com/apache/sedona-db/pull/70#discussion_r2343731547
##########
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` should be qualified as `std::f64::consts::PI` or imported
explicitly at the top of the file to improve code clarity and avoid potential
naming conflicts.
##########
benchmarks/test_bench_base.py:
##########
@@ -27,13 +27,13 @@ def setup_class(self):
num_geoms = 100_000
# Setup tables
- for name, options in [
+ for name, base_options in [
(
"segments_large",
{
"geom_type": "LineString",
"target_rows": num_geoms,
Review Comment:
The change from `[2, 2]` to `[2, 10]` for vertices_per_linestring_range
significantly increases complexity variation. This should be documented in the
code or commit message as it affects benchmark consistency and comparability
with previous results.
```suggestion
"target_rows": num_geoms,
# NOTE: Changed from [2, 2] to [2, 10] to increase
complexity variation.
# This affects benchmark consistency and comparability
with previous results.
```
--
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]
