From 687e1e5f9e75cfd582192533623d1575520bb400 Mon Sep 17 00:00:00 2001 From: Pranav Walimbe Date: Mon, 24 Aug 2026 19:06:31 -0700 Subject: [PATCH 1/3] perf: fill one counting grid for both point statistics collect_points made two serial passes over every point. estimate_distribution scattered into a sqrt(n) cell grid, which is n cells and so 12 MB at SF1 and 120 MB at SF10, then build_histogram scattered again into 32x32. Both now come from one parallel pass over a fixed 256x256 grid. At 256 KiB it stays in cache and is cheap enough to hold per thread, so the fill is a rayon fold and reduce with no atomics. Folding blocks of 8 gives the histogram exactly, since flooring at 256 then dividing by 8 lands on the same cell as flooring at 32. Distribution now uses Morisita's index of dispersion rather than a coefficient of variation. The old test needed a sqrt(n) grid to pin mean occupancy at 1.0, because raw CV depends on how crowded a cell is. Morisita's index reads 1.0 for a Poisson field at any grid size or point count, which is what frees the grid to be fixed and small. Statistics on real pickup points, best of 7: points before after delta 100,000 0.0006 0.0006 -5.9% 1,000,000 0.0061 0.0021 -66.5% 3,000,000 0.0224 0.0039 -82.7% The histogram drives early exits that return an empty result, so a cell that wrongly reads empty is a wrong answer and not a slow one. A differential run of 1000 queries over 3M real points, 515 of them landing in empty space, returns identical answers on both builds, and folded_histogram_matches_direct_binning holds the exactness in a test. --- src/stats/collector.rs | 214 ++++++++++++++++++++++++++++++----------- 1 file changed, 159 insertions(+), 55 deletions(-) diff --git a/src/stats/collector.rs b/src/stats/collector.rs index d0f44a5..712005c 100644 --- a/src/stats/collector.rs +++ b/src/stats/collector.rs @@ -7,6 +7,18 @@ use crate::stats::types::{ DatasetStats, Distribution, GeometryKind, SpatialHistogram, HISTOGRAM_RESOLUTION, }; +/// Per-axis cell count of the counting grid +const COUNT_RESOLUTION: usize = 256; + +/// Cells of the counting grid per histogram cell along one axis +const HISTOGRAM_STRIDE: usize = COUNT_RESOLUTION / HISTOGRAM_RESOLUTION; + +/// Morisita index above which a dataset counts as clustered +const CLUSTERED_INDEX: f64 = 1.5; + +/// Point count below which the distribution is left Unknown +const MIN_POINTS_FOR_DISTRIBUTION: usize = 20; + /// Collect statistics from a flat point coordinate dataset pub fn collect_points(xs: &[f64], ys: &[f64]) -> DatasetStats { let n = xs.len(); @@ -22,7 +34,6 @@ pub fn collect_points(xs: &[f64], ys: &[f64]) -> DatasetStats { } let extent = compute_extent(xs, ys); - let distribution = estimate_distribution(xs, ys, &extent); let mean_density = extent .map(|e| { let area = (e.max().x - e.min().x) * (e.max().y - e.min().y); @@ -33,7 +44,23 @@ pub fn collect_points(xs: &[f64], ys: &[f64]) -> DatasetStats { } }) .unwrap_or(0.0); - let histogram = extent.map(|e| build_histogram(xs, ys, &e)); + + // One parallel pass fills the counting grid behind both answers + let counts = extent.map(|e| count_grid(xs, ys, &e)); + let distribution = match (&counts, &extent) { + (Some(counts), Some(_)) if n >= MIN_POINTS_FOR_DISTRIBUTION => { + if morisita_index(counts, n) > CLUSTERED_INDEX { + Distribution::Clustered + } else { + Distribution::Uniform + } + } + _ => Distribution::Unknown, + }; + let histogram = counts + .as_deref() + .zip(extent) + .map(|(counts, e)| fold_to_histogram(counts, &e)); DatasetStats { n, @@ -117,70 +144,76 @@ fn compute_extent(xs: &[f64], ys: &[f64]) -> Option> { } } -// Grid-based coefficient-of-variation test. CV > 1.5 → Clustered, otherwise Uniform -fn estimate_distribution(xs: &[f64], ys: &[f64], extent: &Option>) -> Distribution { - let n = xs.len(); - if n < 20 { - return Distribution::Unknown; - } - let ext = match extent { - Some(e) => e, - None => return Distribution::Unknown, - }; - let w = ext.max().x - ext.min().x; - let h = ext.max().y - ext.min().y; - if w <= 0.0 || h <= 0.0 { - return Distribution::Unknown; - } - - let grid_dim = (n as f64).sqrt().max(4.0) as usize; - let mut counts = vec![0u32; grid_dim * grid_dim]; - for (&x, &y) in xs.iter().zip(ys.iter()) { - let cx = ((x - ext.min().x) / w * grid_dim as f64) - .min(grid_dim as f64 - 1.0) - .max(0.0) as usize; - let cy = ((y - ext.min().y) / h * grid_dim as f64) - .min(grid_dim as f64 - 1.0) - .max(0.0) as usize; - counts[cy * grid_dim + cx] += 1; - } +// Bin every point into a COUNT_RESOLUTION grid +fn count_grid(xs: &[f64], ys: &[f64], extent: &Rect) -> Vec { + let cells = COUNT_RESOLUTION * COUNT_RESOLUTION; + let w = (extent.max().x - extent.min().x).max(f64::EPSILON); + let h = (extent.max().y - extent.min().y).max(f64::EPSILON); + let cell_w = w / COUNT_RESOLUTION as f64; + let cell_h = h / COUNT_RESOLUTION as f64; + let (min_x, min_y) = (extent.min().x, extent.min().y); - let mean = n as f64 / (grid_dim * grid_dim) as f64; - let variance: f64 = counts - .iter() - .map(|&c| (c as f64 - mean).powi(2)) - .sum::() - / (grid_dim * grid_dim) as f64; - let cv = variance.sqrt() / mean; + // One chunk per thread bounds the number of 256 KiB accumulators + let chunk = (xs.len() / rayon::current_num_threads().max(1)).max(1 << 14); + xs.par_chunks(chunk) + .zip(ys.par_chunks(chunk)) + .map(|(chunk_xs, chunk_ys)| { + let mut local = vec![0u32; cells]; + for (&x, &y) in chunk_xs.iter().zip(chunk_ys.iter()) { + let col = ((x - min_x) / cell_w) + .floor() + .clamp(0.0, (COUNT_RESOLUTION - 1) as f64) as usize; + let row = ((y - min_y) / cell_h) + .floor() + .clamp(0.0, (COUNT_RESOLUTION - 1) as f64) as usize; + local[row * COUNT_RESOLUTION + col] += 1; + } + local + }) + .reduce( + || vec![0u32; cells], + |mut a, b| { + for (x, y) in a.iter_mut().zip(b.iter()) { + *x += y; + } + a + }, + ) +} - if cv > 1.5 { - Distribution::Clustered - } else { - Distribution::Uniform +// Q * sum n_i(n_i - 1) / (N(N - 1)) reads 1.0 for a Poisson field at any grid size +fn morisita_index(counts: &[u32], n: usize) -> f64 { + if n < 2 { + return 1.0; } + let pairs: f64 = counts + .iter() + .map(|&count| { + let count = count as f64; + count * (count - 1.0) + }) + .sum(); + let total = n as f64; + counts.len() as f64 * pairs / (total * (total - 1.0)) } -fn build_histogram(xs: &[f64], ys: &[f64], extent: &Rect) -> SpatialHistogram { +// Flooring at 256 then dividing by 8 hits the same cell as flooring at 32 +fn fold_to_histogram(counts: &[u32], extent: &Rect) -> SpatialHistogram { let w = (extent.max().x - extent.min().x).max(f64::EPSILON); let h = (extent.max().y - extent.min().y).max(f64::EPSILON); - let cell_w = w / HISTOGRAM_RESOLUTION as f64; - let cell_h = h / HISTOGRAM_RESOLUTION as f64; - let mut counts = vec![0u32; HISTOGRAM_RESOLUTION * HISTOGRAM_RESOLUTION]; - for (&x, &y) in xs.iter().zip(ys.iter()) { - let col = ((x - extent.min().x) / cell_w) - .floor() - .clamp(0.0, (HISTOGRAM_RESOLUTION - 1) as f64) as usize; - let row = ((y - extent.min().y) / cell_h) - .floor() - .clamp(0.0, (HISTOGRAM_RESOLUTION - 1) as f64) as usize; - counts[row * HISTOGRAM_RESOLUTION + col] += 1; + let mut folded = vec![0u32; HISTOGRAM_RESOLUTION * HISTOGRAM_RESOLUTION]; + for row in 0..COUNT_RESOLUTION { + for col in 0..COUNT_RESOLUTION { + folded[(row / HISTOGRAM_STRIDE) * HISTOGRAM_RESOLUTION + col / HISTOGRAM_STRIDE] += + counts[row * COUNT_RESOLUTION + col]; + } } SpatialHistogram { - counts, + counts: folded, min_x: extent.min().x, min_y: extent.min().y, - cell_w, - cell_h, + cell_w: w / HISTOGRAM_RESOLUTION as f64, + cell_h: h / HISTOGRAM_RESOLUTION as f64, } } @@ -333,6 +366,77 @@ mod tests { assert_eq!(total as usize, stats.n); } + // Deterministic pseudo-random points reproduce a failure without a rand dependency + fn scattered(n: usize) -> (Vec, Vec) { + let mut state = 0x2545_F491_4F6C_DD1Du64; + let mut next = || { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + (state >> 11) as f64 / (1u64 << 53) as f64 + }; + (0..n).map(|_| (next() * 100.0, next() * 100.0)).unzip() + } + + // Bin straight into HISTOGRAM_RESOLUTION cells + fn direct_histogram(xs: &[f64], ys: &[f64], extent: &Rect) -> Vec { + let w = (extent.max().x - extent.min().x).max(f64::EPSILON); + let h = (extent.max().y - extent.min().y).max(f64::EPSILON); + let cell_w = w / HISTOGRAM_RESOLUTION as f64; + let cell_h = h / HISTOGRAM_RESOLUTION as f64; + let mut counts = vec![0u32; HISTOGRAM_RESOLUTION * HISTOGRAM_RESOLUTION]; + for (&x, &y) in xs.iter().zip(ys.iter()) { + let col = ((x - extent.min().x) / cell_w) + .floor() + .clamp(0.0, (HISTOGRAM_RESOLUTION - 1) as f64) as usize; + let row = ((y - extent.min().y) / cell_h) + .floor() + .clamp(0.0, (HISTOGRAM_RESOLUTION - 1) as f64) as usize; + counts[row * HISTOGRAM_RESOLUTION + col] += 1; + } + counts + } + + #[test] + fn folded_histogram_matches_direct_binning() { + // Early exits read a zero cell as proof a region is empty + let (xs, ys) = scattered(20_000); + let extent = compute_extent(&xs, &ys).expect("scattered points have an extent"); + let folded = fold_to_histogram(&count_grid(&xs, &ys, &extent), &extent); + assert_eq!(folded.counts, direct_histogram(&xs, &ys, &extent)); + } + + #[test] + fn folded_histogram_counts_every_point() { + let (xs, ys) = scattered(5_000); + let stats = collect_points(&xs, &ys); + let total: u32 = stats.histogram.expect("histogram").counts.iter().sum(); + assert_eq!(total as usize, stats.n); + } + + #[test] + fn morisita_index_is_one_for_scattered_points() { + let (xs, ys) = scattered(200_000); + let extent = compute_extent(&xs, &ys).expect("scattered points have an extent"); + let index = morisita_index(&count_grid(&xs, &ys, &extent), xs.len()); + assert!( + (index - 1.0).abs() < 0.1, + "scattered points should sit at the Poisson null, got {index}" + ); + } + + #[test] + fn scattered_points_are_not_clustered() { + let (xs, ys) = scattered(200_000); + assert_eq!(collect_points(&xs, &ys).distribution, Distribution::Uniform); + } + + #[test] + fn distribution_is_unknown_below_the_point_floor() { + let (xs, ys) = scattered(MIN_POINTS_FOR_DISTRIBUTION - 1); + assert_eq!(collect_points(&xs, &ys).distribution, Distribution::Unknown); + } + #[test] fn histogram_skewed_selectivity_beats_area_ratio() { let (xs, ys) = clustered_25(); From 5ed31bb908ca110f62c6f3e3af795e8f007722f5 Mon Sep 17 00:00:00 2001 From: Pranav Walimbe Date: Mon, 24 Aug 2026 19:06:31 -0700 Subject: [PATCH 2/3] perf: filter q2's zone scan down to the one boundary it uses q2 scanned z_name and z_boundary for every zone, collected them, then filtered to Coconino County in Python and kept one row. The pinned reference SQL already puts the filter inside the subquery, so pushing it into scan_parquet matches the query rather than diverging from it. Polars prefilters, reading the predicate column and decoding z_boundary only for surviving rows. Measured on a fixture built to SF1's zone layout, six files of one row group each: 0.512s to 0.155s and a 2273 MiB peak to 1275 MiB, so -70% wall and -44% peak. --- bench/spatial_bench/queries/pycanopy/q02.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bench/spatial_bench/queries/pycanopy/q02.py b/bench/spatial_bench/queries/pycanopy/q02.py index 74bf7fa..b8ef567 100644 --- a/bench/spatial_bench/queries/pycanopy/q02.py +++ b/bench/spatial_bench/queries/pycanopy/q02.py @@ -17,19 +17,20 @@ def pycanopy(data_paths: dict[str, str]) -> pl.DataFrame: + # The zone filter belongs in the scan so Polars decodes one boundary instead of every one zone, trip = pl.collect_all( [ - pl.scan_parquet(data_paths["zone"], storage_options=STORAGE_OPTIONS).select( - ["z_name", "z_boundary"] - ), + pl.scan_parquet(data_paths["zone"], storage_options=STORAGE_OPTIONS) + .filter(pl.col("z_name") == ZONE_NAME) + .select(["z_name", "z_boundary"]) + .head(1), pl.scan_parquet(data_paths["trip"], storage_options=STORAGE_OPTIONS).select( ["t_pickuploc"] ), ] ) - target = zone.filter(pl.col("z_name") == ZONE_NAME).head(1) # from_wkb keeps a MultiPolygon whole rather than exploding it into parts - poly = shapely.from_wkb(target["z_boundary"][0]) + poly = shapely.from_wkb(zone["z_boundary"][0]) sf = SpatialFrame.from_wkb_points(trip, "t_pickuploc") # Only the count is needed, so take the engine's matching indices directly and skip From 49da8c1f38b791aadc01f29cd5ca85083aba40fb Mon Sep 17 00:00:00 2001 From: Pranav Walimbe Date: Mon, 24 Aug 2026 19:16:33 -0700 Subject: [PATCH 3/3] style: state comments directly instead of trailing a justification clause 53 comments across the codebase followed the shape 'fact, so consequence' or 'fact, which explains it'. The clause after the comma restated what the code already showed or padded a claim the reader did not need. Each one now states its fact and stops. Where the trailing clause carried real information it moves into the sentence or onto its own line rather than hanging off a comma. Comments only. No code changed. --- bench/spatial_bench/__main__.py | 2 +- bench/spatial_bench/bootstrap.sh | 6 +-- bench/spatial_bench/config.py | 4 +- bench/spatial_bench/driver_utils.py | 2 +- bench/spatial_bench/queries/pycanopy/q02.py | 3 +- bench/spatial_bench/queries/pycanopy/q04.py | 2 +- bench/spatial_bench/queries/pycanopy/q11.py | 2 +- bench/spatial_bench/report_utils.py | 4 +- python/pycanopy/coordinates.py | 4 +- python/pycanopy/engine.py | 4 +- python/pycanopy/executor.py | 12 +++--- python/pycanopy/optimizer.py | 4 +- src/index/brute.rs | 8 ++-- src/index/rtree.rs | 9 +++-- src/lib.rs | 6 +-- src/planner/selector.rs | 4 +- src/query/batch.rs | 40 +++++++++---------- src/query/geodesy.rs | 8 ++-- src/query/prepared.rs | 2 +- src/stats/collector.rs | 6 +-- src/stats/types.rs | 3 +- src/wkb.rs | 4 +- tests/python/test_delta.py | 6 +-- tests/python/test_engine.py | 6 +-- tests/python/test_frame.py | 8 ++-- tests/python/test_geometry_ops.py | 2 +- tests/python/test_joins.py | 12 +++--- .../python/test_spatial_bench_multi_engine.py | 4 +- tests/python/test_spatial_bench_profile.py | 2 +- 29 files changed, 89 insertions(+), 90 deletions(-) diff --git a/bench/spatial_bench/__main__.py b/bench/spatial_bench/__main__.py index 0273e7b..e94ea31 100644 --- a/bench/spatial_bench/__main__.py +++ b/bench/spatial_bench/__main__.py @@ -343,7 +343,7 @@ def main(argv: list[str] | None = None) -> int: else: produced = [] - # A failing released baseline is a result, so only the branch build is required + # A failing released baseline is a result and only the branch build is required required = ["branch"] if args.profile else list(statuses) if not all(statuses.get(node, False) for node in required) or not produced: logs = [p for p in paths if p.suffix == ".log"] diff --git a/bench/spatial_bench/bootstrap.sh b/bench/spatial_bench/bootstrap.sh index 7210233..c04384d 100644 --- a/bench/spatial_bench/bootstrap.sh +++ b/bench/spatial_bench/bootstrap.sh @@ -76,8 +76,8 @@ log "installing uv" curl -LsSf https://astral.sh/uv/install.sh | sh source "$HOME/.local/bin/env" -# Amazon Linux 2023 ships Python 3.9, below the project floor, so pin uv to a managed -# 3.10 for every sync and run. It is the supported floor, so the box exercises it. +# Amazon Linux 2023 ships Python 3.9 below the project floor +# Pin uv to a managed 3.10 for every sync and run which is the supported floor export UV_PYTHON=3.10 log "cloning ${REPO_URL} @ ${REPO_BRANCH}" @@ -125,7 +125,7 @@ fi mkdir -p /data/scratch /opt/pycanopy/assets -# /tmp is tmpfs (RAM) on Amazon Linux 2023, so spill out-of-core scratch and Polars sort to the EBS data volume. +# /tmp is tmpfs on Amazon Linux 2023 so scratch and Polars sort spill to the EBS data volume export PYCANOPY_SCRATCH=/data/scratch export POLARS_TEMP_DIR=/data/scratch export TMPDIR=/data/scratch diff --git a/bench/spatial_bench/config.py b/bench/spatial_bench/config.py index 73cd449..f364830 100644 --- a/bench/spatial_bench/config.py +++ b/bench/spatial_bench/config.py @@ -31,8 +31,8 @@ STORAGE_OPTIONS = {"skip_signature": "true"} -# Mirror of the upstream Hugging Face dataset, which is what the committed answers were -# generated from. Public read, so queries stay anonymous. +# Mirror of the upstream Hugging Face dataset the committed answers were generated from +# Public read keeps queries anonymous PUBLIC_DATA_ROOT = "s3://pycanopy-bench-data/spatialbench" PUBLIC_DATA_TEMPLATE = f"{PUBLIC_DATA_ROOT}/{DATASET_VERSION}/sf{{scale_factor}}" diff --git a/bench/spatial_bench/driver_utils.py b/bench/spatial_bench/driver_utils.py index c663e4f..2b85f73 100644 --- a/bench/spatial_bench/driver_utils.py +++ b/bench/spatial_bench/driver_utils.py @@ -239,7 +239,7 @@ def run_profile_suite(query_ids: list[str], data_dir: str, variant: str = "branc for query_id, result in results.items() if result["status"] != "ok" or result.get("verify") != "match" ] - # The released build is a baseline, so only the branch build has to verify clean + # The released build is a baseline and only the branch build has to verify clean if invalid and variant == "branch": raise RuntimeError(f"profile verification failed for: {', '.join(invalid)}") if invalid: diff --git a/bench/spatial_bench/queries/pycanopy/q02.py b/bench/spatial_bench/queries/pycanopy/q02.py index b8ef567..3c2c901 100644 --- a/bench/spatial_bench/queries/pycanopy/q02.py +++ b/bench/spatial_bench/queries/pycanopy/q02.py @@ -33,7 +33,6 @@ def pycanopy(data_paths: dict[str, str]) -> pl.DataFrame: poly = shapely.from_wkb(zone["z_boundary"][0]) sf = SpatialFrame.from_wkb_points(trip, "t_pickuploc") - # Only the count is needed, so take the engine's matching indices directly and skip - # gathering the in-zone rows into a DataFrame. + # Only the count is needed so take the engine's matching indices without gathering rows idx = sf.engine.points_within_distance_of_polygon(poly, 0.0) return pl.DataFrame({"trip_count_in_coconino_county": [len(idx)]}) diff --git a/bench/spatial_bench/queries/pycanopy/q04.py b/bench/spatial_bench/queries/pycanopy/q04.py index ca3c5fb..fc3bb59 100644 --- a/bench/spatial_bench/queries/pycanopy/q04.py +++ b/bench/spatial_bench/queries/pycanopy/q04.py @@ -28,7 +28,7 @@ def pycanopy(data_paths: dict[str, str]) -> pl.DataFrame: ), ] ) - # Only the top trips need their geometry, so this second scan runs after the first result + # Only the top trips need geometry and this second scan runs after the first result trip = ( trip_scan.select(["t_tripkey", "t_pickuploc"]) .filter(pl.col("t_tripkey").is_in(top["t_tripkey"].implode())) diff --git a/bench/spatial_bench/queries/pycanopy/q11.py b/bench/spatial_bench/queries/pycanopy/q11.py index 51df9fe..45a0519 100644 --- a/bench/spatial_bench/queries/pycanopy/q11.py +++ b/bench/spatial_bench/queries/pycanopy/q11.py @@ -46,7 +46,7 @@ def pycanopy(data_paths: dict[str, str]) -> pl.DataFrame: .collect_batched() ) - # Aligned morsels carry the same trips on each side, so per-morsel counts sum to the global count + # Aligned morsels carry the same trips on each side and per-morsel counts sum to the global count count = 0 for pickup, dropoff in zip(pickup_batches, dropoff_batches, strict=True): count += ( diff --git a/bench/spatial_bench/report_utils.py b/bench/spatial_bench/report_utils.py index f54d31c..5cd3605 100644 --- a/bench/spatial_bench/report_utils.py +++ b/bench/spatial_bench/report_utils.py @@ -417,7 +417,7 @@ def _profile_head() -> str: def _metadata_block(metadata: dict[str, str]) -> str: - # Run metadata, so a later reader can tell a code change from a machine change + # Run metadata lets a later reader tell a code change from a machine change lines = [_SEP, "Run metadata", _SUBSEP] lines.extend(f"{key:<22}{value}" for key, value in metadata.items()) return "\n".join(lines) @@ -467,7 +467,7 @@ def _reports_metrics(results: dict) -> bool: def _stage_order(stages: list[dict[str, float]]) -> list[str]: - # Union both builds' stages, keeping pipeline order and appending whatever only one has + # Union both builds' stages in pipeline order and append whatever only one has ordered: list[str] = [] for stage in stages: for name in stage: diff --git a/python/pycanopy/coordinates.py b/python/pycanopy/coordinates.py index d849752..fb9425f 100644 --- a/python/pycanopy/coordinates.py +++ b/python/pycanopy/coordinates.py @@ -41,7 +41,7 @@ def resolve_coordinate_system( raise ValueError( f"coordinate_system must be 'planar' or 'geographic', got {coordinate_system!r}" ) - # Only geographic constrains its coordinates, so a planar frame skips the scan entirely + # Only geographic constrains its coordinates and a planar frame skips the scan entirely if coordinate_system == "geographic" and len(xs) > 0 and not _looks_geographic(xs, ys): warnings.warn( "coordinate_system='geographic' reads x/y as WGS84 lon/lat degrees, but these " @@ -55,5 +55,5 @@ def resolve_coordinate_system( def _looks_geographic(xs: np.ndarray, ys: np.ndarray) -> bool: - # Lon/lat degrees are bounded, so anything outside those bounds cannot be WGS84 + # Lon/lat degrees are bounded and anything outside them cannot be WGS84 return bool(np.all(np.abs(xs) <= _MAX_ABS_LON) and np.all(np.abs(ys) <= _MAX_ABS_LAT)) diff --git a/python/pycanopy/engine.py b/python/pycanopy/engine.py index b9fa594..aca557b 100644 --- a/python/pycanopy/engine.py +++ b/python/pycanopy/engine.py @@ -307,8 +307,8 @@ def _wkb_points_fast(arr: pa.Array) -> tuple[np.ndarray, np.ndarray] | None: if offsets_buf is None or data_buf is None: return None - # Offsets are int32 for binary, int64 for large_binary (what polars emits). A - # sliced array shares its parent's buffers, so index past the slice's offset. + # Offsets are int32 for binary and int64 for large_binary (what polars emits) + # A sliced array shares its parent's buffers so index past the slice's offset offset_dtype = " pl.Expr: - # Bounding-box filter via map_batches. is_elementwise=False is a barrier, so the closure - # sees only post-scalar-filter rows, masked against the global index in Rust. + # Bounding-box filter via map_batches with is_elementwise=False as a barrier + # The closure sees only post-scalar-filter rows masked against the global index in Rust def _apply(s: pl.Series) -> pl.Series: orig_idx = s.to_numpy() if len(orig_idx) == 0: @@ -200,8 +200,8 @@ def _execute_body( has_joins = any(isinstance(n, _JOIN_TYPES) for n in plan) - # Large-probe joins stream the probe in morsels and concatenate, so the join - # intermediate is bounded by one morsel rather than the full result. + # Large-probe joins stream the probe in morsels and concatenate + # The join intermediate is bounded by one morsel rather than the full result if has_joins: morsel = batch_size if batch_size is not None else MORSEL_ROWS join_node = next(n for n in plan if isinstance(n, _JOIN_TYPES)) @@ -415,8 +415,8 @@ def _emit_fused( def _emit_points_within_distance_of_polygon( self, node: PointsWithinDistanceOfPolygonNode, sf, lf: pl.LazyFrame ) -> pl.LazyFrame: - # Keep points within node.distance of the query polygon. The polygon is queried - # against all points, so indices resolve once and the lf filters by original row. + # Keep points within node.distance of the query polygon + # It is queried against all points so indices resolve once and lf filters by original row indices = sf.engine.points_within_distance_of_polygon(node.polygon, node.distance) return self._filter_by_indices(lf, indices) diff --git a/python/pycanopy/optimizer.py b/python/pycanopy/optimizer.py index 8516f0e..f39dbc1 100644 --- a/python/pycanopy/optimizer.py +++ b/python/pycanopy/optimizer.py @@ -133,7 +133,7 @@ def optimize(self, plan: Plan, engine) -> Plan: """ if not plan: return plan - # A trailing SelectNode is a terminal projection, not a predicate, so the cost passes skip it + # A trailing SelectNode is a terminal projection rather than a predicate and cost passes skip it select_tail = None if isinstance(plan[-1], SelectNode): select_tail = plan[-1] @@ -200,7 +200,7 @@ def _disk_selectivity( return min(1.0, self._disk_area(node, coordinate_system) / total_area) def _disk_area(self, node: WithinDistanceOfPointNode, coordinate_system: str) -> float: - # A geographic radius is meters against a degree extent, so it becomes a degree ellipse + # A geographic radius is meters against a degree extent and becomes a degree ellipse if coordinate_system != "geographic": return math.pi * node.distance * node.distance cos_lat = math.cos(math.radians(min(abs(node.cy), _POLE_GUARD_LAT))) diff --git a/src/index/brute.rs b/src/index/brute.rs index 96d8e50..b58134f 100644 --- a/src/index/brute.rs +++ b/src/index/brute.rs @@ -31,7 +31,7 @@ impl SpatialIndex for BruteForce { } fn nearest(&self, qx: f64, qy: f64, k: usize) -> Vec { - // Rank by point-to-MBR distance, which is exact for the degenerate boxes of a point + // Rank by point-to-MBR distance which is exact for the degenerate boxes of a point // dataset and the lower bound the polygon refinement pass needs. let n = self.bbox_min_x.len(); let k = k.min(n); @@ -181,9 +181,9 @@ mod tests { #[test] fn polygon_nearest_ranks_by_mbr_not_centroid() { - // A wide sliver spanning x 0..10 at y 0, and a small square at x 4..5, y 3..4. From - // (5, 0.5) the sliver's edge is 0.5 away but its centroid is 5 away, which centroid - // ranking put second. Ranking by MBR is the lower bound polygon refinement relies on. + // A wide sliver spans x 0..10 at y 0 and a small square sits at x 4..5 y 3..4 + // From (5, 0.5) the sliver's edge is 0.5 away while its centroid is 5 away + // Centroid ranking put it second and MBR is the lower bound refinement relies on let xs = vec![ 0.0, 10.0, 10.0, 0.0, 0.0, // sliver ring 4.0, 5.0, 5.0, 4.0, 4.0, // square ring diff --git a/src/index/rtree.rs b/src/index/rtree.rs index 8f742bd..9f37169 100644 --- a/src/index/rtree.rs +++ b/src/index/rtree.rs @@ -38,8 +38,9 @@ impl Ord for NeighborNode { } thread_local! { - // Reused across every query on this worker. geo-index's own `neighbors` and `search` allocate - // a fresh queue and result vector per call, which dominates a kernel invoked once per query. + // Reused across every query on this worker + // geo-index's own `neighbors` and `search` allocate a fresh queue and vector per call + // That allocation dominates a kernel invoked once per query static NEIGHBOR_QUEUE: RefCell>> = const { RefCell::new(BinaryHeap::new()) }; static SEARCH_QUEUE: RefCell> = const { RefCell::new(VecDeque::new()) }; @@ -258,7 +259,7 @@ mod tests { assert_eq!(sorted(build(xs, ys).range(0.5, 0.5, 1.5, 1.5)), vec![4]); } - // Deterministic pseudo-random points, so a failure reproduces without a rand dependency + // Deterministic pseudo-random points reproduce a failure without a rand dependency fn scattered(n: usize) -> (Vec, Vec) { let mut state = 0x2545_F491_4F6C_DD1Du64; let mut next = || { @@ -315,7 +316,7 @@ mod tests { #[test] fn nearest_into_handles_exact_distance_ties() { - // Four coincident points: every candidate ties, so only the set is well defined + // Four coincident points tie on every candidate and only the set is well defined let tree = build(vec![1.0, 1.0, 1.0, 1.0, 9.0], vec![1.0, 1.0, 1.0, 1.0, 9.0]); let mut got = Vec::new(); tree.nearest_into(1.0, 1.0, 3, &mut got); diff --git a/src/lib.rs b/src/lib.rs index 6b9c025..2208538 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -954,7 +954,7 @@ impl Engine { )); } let started = Instant::now(); - // Geographic distances are meters over degrees, so the box has to be widened, not dilated + // Geographic distances are meters over degrees and need the box widened not dilated let (min_x, min_y, max_x, max_y) = match self.metric { DistanceMetric::Planar => (cx - distance, cy - distance, cx + distance, cy + distance), DistanceMetric::Haversine => conservative_degree_box(cx, cy, distance), @@ -2241,8 +2241,8 @@ impl Engine { } let ring_off = self.ring_offsets.as_deref().unwrap(); let poly_off = self.poly_offsets.as_deref().unwrap(); - // For MultiPolygons the pair area is the sum over their part pairs: parts within - // one polygon are disjoint, so the part intersections partition the overlap. + // For MultiPolygons the pair area sums over their part pairs + // Parts within one polygon are disjoint and their intersections partition the overlap let areas: Vec = match &self.part_poly { Some(pp) => { let (offsets, parts) = polygon_parts_csr(pp, self.n_polygons); diff --git a/src/planner/selector.rs b/src/planner/selector.rs index 2648afa..1565c09 100644 --- a/src/planner/selector.rs +++ b/src/planner/selector.rs @@ -291,7 +291,7 @@ mod tests { #[test] fn auto_builds_index_for_many_probes() { - // Many probes amortise the build, so the index wins + // Many probes amortise the build and the index wins let s = stats(1_000_000, GeometryKind::Point, Distribution::Clustered); let f = CostFactors::default(); assert_eq!( @@ -312,7 +312,7 @@ mod tests { #[test] fn best_available_reuses_built_index_for_few_probes() { - // Build cost already paid, so even 1 probe uses the built index + // Build cost already paid so even 1 probe uses the built index let s = stats(1_000_000, GeometryKind::Point, Distribution::Clustered); let f = CostFactors::default(); assert_eq!( diff --git a/src/query/batch.rs b/src/query/batch.rs index ad77911..b4ed039 100644 --- a/src/query/batch.rs +++ b/src/query/batch.rs @@ -279,7 +279,7 @@ pub fn par_radius( distance: f64, metric: DistanceMetric, ) -> Vec { - // Dispatch once per call, never per candidate, so the planar path compiles as it always did + // Dispatch once per call rather than per candidate and the planar path compiles unchanged match metric { DistanceMetric::Planar => { let d2 = distance * distance; @@ -320,7 +320,7 @@ pub fn par_within_distance( distance: f64, metric: DistanceMetric, ) -> Vec { - // Dispatch once per call, never per candidate, so the planar path compiles as it always did + // Dispatch once per call rather than per candidate and the planar path compiles unchanged match metric { DistanceMetric::Planar => within_distance_planar(index, qxs, qys, xs, ys, distance), DistanceMetric::Haversine => within_distance_haversine(index, qxs, qys, xs, ys, distance), @@ -421,7 +421,7 @@ pub fn par_within_distance_flipped( ) -> Vec { // Build a KD-tree on the (smaller) query side let q_index = PackedKdTree::build(Arc::from(qxs.to_vec()), Arc::from(qys.to_vec())); - // Dispatch once per call, never per candidate, so the planar path compiles as it always did + // Dispatch once per call rather than per candidate and the planar path compiles unchanged match metric { DistanceMetric::Planar => { let d2 = distance * distance; @@ -651,7 +651,7 @@ fn knn_polys_exact( scratch: &mut KnnScratch, ) { let kept = &mut scratch.kept; - // Seed pass. Parts can share a logical polygon, so grow the fetch until k distinct polygons + // Seed pass grows the fetch until k distinct polygons because parts can share one polygon // are held or every part has been seen. let mut fetch = (k + SEED_MARGIN).min(n_parts.max(1)); loop { @@ -667,7 +667,7 @@ fn knn_polys_exact( fetch = fetch.saturating_mul(2).min(n_parts); } if kept.len() < k { - // The seed swept every part, so the dataset holds fewer than k polygons + // The seed swept every part and the dataset holds fewer than k polygons kept.resize(k, (u32::MAX, f64::INFINITY)); return; } @@ -683,15 +683,15 @@ fn knn_polys_exact( return; } - // Sweep pass over every part the seed's radius admits, nearest MBR first. The seed's k stay - // in `kept` as the working bound, so the sweep can only tighten the answer, never lose it to - // a rounding edge where a polygon's exact distance and its MBR bound differ by an ulp. + // Sweep pass over every part the seed's radius admits nearest MBR first + // The seed's k stay in `kept` as the working bound and the sweep only tightens the answer + // It cannot lose to a rounding edge where exact distance and MBR bound differ by an ulp index.range_into(qx - kth, qy - kth, qx + kth, qy + kth, &mut scratch.sweep); let seeds = &scratch.seeds; let cands = &mut scratch.cands; cands.clear(); - // The seed already refined its own parts, so re-measuring them would repeat k exact - // distances per query. Seeds are the MBR-nearest handful, so a scan beats a set. + // The seed already refined its own parts and re-measuring repeats k exact distances + // Seeds are the MBR-nearest handful where a scan beats a set cands.extend( scratch .sweep @@ -702,8 +702,8 @@ fn knn_polys_exact( cands.sort_unstable_by(|a, b| a.1.partial_cmp(&b.1).unwrap_or(Ordering::Equal)); for &(ei, lb_sq) in cands.iter() { - // Candidates arrive nearest-MBR-first, so once a lower bound cannot beat the kth exact - // distance held, nothing after it can either. + // Candidates arrive nearest-MBR-first + // Once a lower bound cannot beat the kth exact distance held nothing after it can if lb_sq >= kth_sq { break; } @@ -791,7 +791,7 @@ pub fn par_knn_to_polygons( let tiles = build_query_tiles(qxs, qys, &order, TILE_GRID); // Parallelise over tiles so each tile's polygon vertices stay warm in L3 across its queries. - // Each tile writes its answers into three flat buffers rather than one Vec per query, so the + // Each tile writes its answers into three flat buffers rather than one Vec per query and the // kernel allocates per tile instead of per query. let tile_results: TileResults = tiles .par_iter() @@ -1211,7 +1211,7 @@ mod tests { #[test] fn single_probe_haversine_matches_the_multi_probe_path() { - // The one-query fast path parallelises differently, so it needs its own check + // The one-query fast path parallelises differently and needs its own check let (xs, ys) = lonlat_cloud(); let index = PackedRTree::build(Arc::from(xs.clone()), Arc::from(ys.clone())); let (qx, qy, distance) = (-73.9, 40.7, 300_000.0); @@ -1395,8 +1395,8 @@ mod tests { #[test] fn donut_mbrs_do_not_hide_the_nearest_polygon() { - // Four annuli whose MBRs all contain the query, so MBR rank puts them first, and one - // small square that is genuinely nearest. Fixed 4*k oversampling returned an annulus. + // Four annuli whose MBRs all contain the query and one small square genuinely nearest + // MBR rank puts the annuli first and fixed 4*k oversampling returned one let mut parts: Vec>> = (0..4) .map(|i| donut(i as f64 * 0.1, 0.0, 100.0, 50.0)) .collect(); @@ -1499,8 +1499,8 @@ mod tests { #[test] fn multipolygon_parts_collapse_to_one_neighbour() { - // Polygon 0 has a far part and a near one, polygons 1 and 2 sit between them. k counts - // distinct polygons, so polygon 0 must appear once, at its nearer part's distance. + // Polygon 0 has a far part and a near one with polygons 1 and 2 between them + // k counts distinct polygons so polygon 0 appears once at its nearer part's distance let parts = vec![ vec![square_ring(40.0, 0.0, 1.0)], vec![square_ring(5.0, 0.0, 1.0)], @@ -1595,12 +1595,12 @@ mod tests { ); assert_eq!(q_idx.len(), n * k); - // The sorted path emits globally ordered pairs, so regroup by query to compare + // The sorted path emits globally ordered pairs and needs regrouping by query to compare let mut grouped: Vec> = vec![Vec::new(); n]; for i in 0..q_idx.len() { grouped[q_idx[i] as usize].push((t_idx[i], dists[i])); } - // Only the sorted path promises a tie-break, so normalise both sides to (dist, idx) + // Only the sorted path promises a tie-break and both sides normalise to (dist, idx) let by_dist_then_idx = |v: &mut Vec<(u32, f64)>| { v.sort_unstable_by(|a, b| { a.1.partial_cmp(&b.1) diff --git a/src/query/geodesy.rs b/src/query/geodesy.rs index d0f8dbd..7451b18 100644 --- a/src/query/geodesy.rs +++ b/src/query/geodesy.rs @@ -53,13 +53,13 @@ pub fn conservative_degree_box(cx: f64, cy: f64, distance_m: f64) -> (f64, f64, let max_y = (cy + lat_delta).min(90.0); let all_longitudes = (-180.0, min_y, 180.0, max_y); - // Longitude degrees shrink with cos(lat), so widen at the highest latitude reached, not at cy + // Widen at the highest latitude reached where longitude degrees are shortest let extreme_lat = min_y.abs().max(max_y.abs()); if extreme_lat >= POLE_GUARD_LAT { return all_longitudes; } let lon_delta = distance_m / (METERS_PER_DEGREE_LON_EQUATOR * extreme_lat.to_radians().cos()); - // Running past either edge wraps the antimeridian, which no degree interval can express + // Running past either edge wraps the antimeridian beyond any degree interval if cx - lon_delta < -180.0 || cx + lon_delta > 180.0 { return all_longitudes; } @@ -101,7 +101,7 @@ mod tests { #[test] fn matches_geo_crate_across_latitude_bands() { - // Latitude scales the longitude term by its cosine, so sweep a spread of bands + // Sweep a spread of bands to cover the cosine scaling of the longitude term for lat in [-85.0, -45.0, -1.0, 0.0, 1.0, 45.0, 85.0] { for dlon in [0.001, 0.5, 10.0] { let got = hoisted(0.0, lat, dlon, lat + 0.25); @@ -147,7 +147,7 @@ mod tests { #[test] fn degree_box_spans_all_longitudes_across_the_antimeridian() { - // A box at 179.9E reaching past 180 wraps, which a degree interval cannot express + // A box at 179.9E reaching past 180 wraps beyond what a degree interval expresses let (min_x, _, max_x, _) = conservative_degree_box(179.9, 0.0, 100_000.0); assert_eq!((min_x, max_x), (-180.0, 180.0)); } diff --git a/src/query/prepared.rs b/src/query/prepared.rs index b9428e4..162a494 100644 --- a/src/query/prepared.rs +++ b/src/query/prepared.rs @@ -251,7 +251,7 @@ mod tests { #[test] fn only_probed_parts_are_built() { - // Two ring polygons side by side, so a probe of the first never reaches the second + // Two ring polygons side by side where a probe of the first never reaches the second let (mut xs, mut ys, _, _) = ring_polygon(200); let (right_xs, right_ys, _, _) = ring_polygon(200); xs.extend(right_xs.iter().map(|x| x + 10.0)); diff --git a/src/stats/collector.rs b/src/stats/collector.rs index 712005c..8795141 100644 --- a/src/stats/collector.rs +++ b/src/stats/collector.rs @@ -116,7 +116,7 @@ pub fn collect_polygons( } fn compute_extent(xs: &[f64], ys: &[f64]) -> Option> { - // min/max reduce in parallel: order-independent, so the extent matches a serial fold + // Order-independent min/max reduce in parallel matches a serial fold let init = || { ( f64::INFINITY, @@ -230,8 +230,8 @@ fn build_polygon_centroid_histogram( let cell_h = h / HISTOGRAM_RESOLUTION as f64; let n_polys = poly_offsets.len().saturating_sub(1); let cells = HISTOGRAM_RESOLUTION * HISTOGRAM_RESOLUTION; - // Bin each polygon's exterior-ring centroid in parallel, accumulating into per-thread - // histograms then summing them. Integer sums are order-independent, so counts are exact. + // Bin each polygon's exterior-ring centroid in parallel into per-thread histograms + // Integer sums are order-independent and keep the summed counts exact let counts = (0..n_polys) .into_par_iter() .fold( diff --git a/src/stats/types.rs b/src/stats/types.rs index 2f5be7a..e10b385 100644 --- a/src/stats/types.rs +++ b/src/stats/types.rs @@ -181,8 +181,7 @@ mod tests { #[test] fn histogram_selectivity_quarter_bbox() { // 1024 points uniform, bbox covering bottom-left quarter should return ~0.25. - // Tolerance is 0.05 because exact cell-boundary alignment causes floor() to - // include the boundary cell, giving a small over-count (17x17 vs 16x16 cells). + // Tolerance is 0.05 for the boundary cell that floor() includes (17x17 vs 16x16) let hist = uniform_histogram(HISTOGRAM_RESOLUTION, 1); let bbox = rect(0.0, 0.0, 0.5, 0.5); let sel = hist.selectivity(&bbox, 1024); diff --git a/src/wkb.rs b/src/wkb.rs index 60e4ec3..1abc5fe 100644 --- a/src/wkb.rs +++ b/src/wkb.rs @@ -376,7 +376,7 @@ fn parse_polygons_chunked( }; // SAFETY: every chunk decoded exactly its coordinate count (asserted in fill_chunk) into a - // SAFETY: disjoint slice partitioning [0, total_coords), so all elements are initialised. + // SAFETY: disjoint slice partitioning of [0, total_coords) initialises every element unsafe { xs.set_len(total_coords); ys.set_len(total_coords); @@ -608,7 +608,7 @@ mod tests { #[test] fn unsupported_geometry_type_is_error() { - // Byte order then WKB type 1 (Point), which the polygon decoder rejects + // Byte order then WKB type 1 (Point) which the polygon decoder rejects let mut geom = vec![1u8]; geom.extend_from_slice(&1u32.to_le_bytes()); geom.extend_from_slice(&0.0f64.to_le_bytes()); diff --git a/tests/python/test_delta.py b/tests/python/test_delta.py index dc18001..c239e6e 100644 --- a/tests/python/test_delta.py +++ b/tests/python/test_delta.py @@ -75,9 +75,9 @@ def test_size_cap_flushes_delta(engine): def test_cost_flush_fires(): # Fresh uniform grid so select_index reliably picks Grid (cost threshold = N). - # The shared module engine has accumulated flushed points from prior tests, - # shifting its distribution to Clustered (KD-tree, cost = N*log2 N), which - # would require ~140 queries to trigger — too expensive for a unit test. + # The shared module engine has accumulated flushed points from prior tests + # That shifts its distribution to Clustered on a KD-tree costing N*log2 N + # Triggering it would need ~140 queries which is too expensive for a unit test _n = 529 # 23x23 uniform grid, above the 500 brute-force threshold eng = Engine.from_coords( np.array([float(i % 23) for i in range(_n)], dtype=np.float64), diff --git a/tests/python/test_engine.py b/tests/python/test_engine.py index 32825ff..90c44c5 100644 --- a/tests/python/test_engine.py +++ b/tests/python/test_engine.py @@ -333,7 +333,7 @@ def test_range_empty_returns_empty(engine): def test_radius_refines_to_circle(engine): - # (0,0),(1,0),(0,1) are within 1.0; (1,1) is in the bbox but sqrt(2) away, so dropped + # Three corners are within 1.0 while (1,1) sits in the bbox at sqrt(2) and drops assert sorted(engine.radius_query(0.0, 0.0, 1.0).tolist()) == [0, 1, 3] @@ -391,8 +391,8 @@ def test_from_polygons_stats_contains_n(poly_engine): def test_from_polygons_accepts_multipolygon(): - # SQUARES[1] and SQUARES[2] are disjoint; as one MultiPolygon they are one logical - # polygon, so a point in either part returns the same index, counted once. + # SQUARES[1] and SQUARES[2] are disjoint but form one logical polygon as a MultiPolygon + # A point in either part returns the same index counted once mp = MultiPolygon([SQUARES[1], SQUARES[2]]) eng = Engine.from_polygons([SQUARES[0], mp]) assert eng.contains(0.5, 0.5) == [0] # in the plain polygon diff --git a/tests/python/test_frame.py b/tests/python/test_frame.py index eccfa25..668d74e 100644 --- a/tests/python/test_frame.py +++ b/tests/python/test_frame.py @@ -354,7 +354,7 @@ def test_coordinate_system_rejects_unknown_value(): def test_geographic_radius_query_measures_meters(sf_airports): - # 4000 km reaches LAX (3974) but not SFO (4152), so the threshold lands between them + # 4000 km reaches LAX (3974) but not SFO (4152) and the threshold lands between them result = sf_airports.radius_query(*_JFK, 4_000_000) assert sorted(result["name"].to_list()) == ["JFK", "LAX"] @@ -370,7 +370,7 @@ def test_geographic_within_distance_of_point_lazy_matches_eager(sf_airports): def test_planar_frame_reads_the_same_distance_as_degrees(sf_airports): - # The identical call on a planar frame measures degrees, so 4e6 degrees spans the globe + # The identical call on a planar frame measures degrees where 4e6 spans the globe df = sf_airports.df planar = SpatialFrame(df, "lon", "lat") result = planar.radius_query(*_JFK, 4_000_000) @@ -378,7 +378,7 @@ def test_planar_frame_reads_the_same_distance_as_degrees(sf_airports): def test_geographic_survives_range_filter(sf_airports): - # A derived frame keeps the setting, since it is a fact about the coordinates + # A derived frame keeps the setting as a fact about the coordinates derived = sf_airports.range_filter(-180.0, -90.0, 180.0, 90.0) assert derived.coordinate_system == "geographic" @@ -412,7 +412,7 @@ def test_geographic_does_not_warn_on_lon_lat(sf_airports): def test_planar_never_warns_whatever_the_coordinates_look_like(sf_airports): - # A small planar grid sits inside lon/lat's range, so guessing from the data would misfire + # A small planar grid sits inside lon/lat's range where guessing from the data misfires with warnings.catch_warnings(): warnings.simplefilter("error") SpatialFrame(sf_airports.df, "lon", "lat") diff --git a/tests/python/test_geometry_ops.py b/tests/python/test_geometry_ops.py index 5182c27..ec092f8 100644 --- a/tests/python/test_geometry_ops.py +++ b/tests/python/test_geometry_ops.py @@ -386,7 +386,7 @@ def test_knn_to_polygons_pads_when_k_exceeds_polygon_count(): def test_knn_to_polygons_matches_brute_force_on_mixed_geometry(): - # Concave shapes, holes, and multi-part geometries together, so the seed's MBR bound is wrong + # Concave shapes holes and multi-part geometries together make the seed's MBR bound wrong # often enough to force the sweep, and k is wide enough to keep it running. rng = np.random.default_rng(11) polys: list = [] diff --git a/tests/python/test_joins.py b/tests/python/test_joins.py index 46bff93..b012b0f 100644 --- a/tests/python/test_joins.py +++ b/tests/python/test_joins.py @@ -257,9 +257,9 @@ def test_streamed_within_join_polygons_matches_single_shot(sf_polygons): assert streamed.select(cols).sort(cols).equals(single.select(cols).sort(cols)) -# index_mode "none" forces brute force; results must match the indexed ("eager") -# path. The fixtures have n >= 500 so the default path builds a real index. They are -# module-scoped, so each test restores the mode it changed via _index_mode. +# index_mode "none" forces brute force and results must match the indexed "eager" path +# The fixtures have n >= 500 so the default path builds a real index +# They are module-scoped and each test restores the mode it changed via _index_mode @contextmanager @@ -479,7 +479,7 @@ def _jfk_probe(): def test_geographic_within_distance_join_measures_meters(sf_airports): - # 4000 km reaches LAX (3974) but not SFO (4152), so the threshold lands between them + # 4000 km reaches LAX (3974) but not SFO (4152) and the threshold lands between them out = ( sf_airports.lazy() .within_distance_join(_jfk_probe(), x_col="lon", y_col="lat", distance=4_000_000) @@ -489,7 +489,7 @@ def test_geographic_within_distance_join_measures_meters(sf_airports): def test_geographic_within_distance_join_flipped_path(sf_airports): - # Q=3 > N//2 on the 5-airport frame, so the optimizer indexes the query side instead + # Q=3 > N//2 on the 5-airport frame and the optimizer indexes the query side instead probe = pl.DataFrame( { "probe": ["from_jfk", "from_lhr", "from_sfo"], @@ -515,7 +515,7 @@ def test_geographic_within_distance_join_flipped_path(sf_airports): def test_geographic_within_distance_join_streams_in_morsels(sf_airports): - # Batched collection re-plans per morsel, so the metric has to survive the slicing + # Batched collection re-plans per morsel and the metric must survive the slicing probe = pl.concat([_jfk_probe()] * 4) out = pl.concat( list( diff --git a/tests/python/test_spatial_bench_multi_engine.py b/tests/python/test_spatial_bench_multi_engine.py index b0d48f0..12a3637 100644 --- a/tests/python/test_spatial_bench_multi_engine.py +++ b/tests/python/test_spatial_bench_multi_engine.py @@ -46,8 +46,8 @@ def test_config_matches_spatialbench_single_node_protocol(): assert REGION == "us-west-2" assert INSTANCE_TYPE == "m7i.2xlarge" assert PUBLIC_DATA_TEMPLATE.startswith("s3://") - # The dataset build has to be identifiable from the path, since the committed answers only - # match one of them. + # The dataset build has to be identifiable from the path + # The committed answers match only one of them assert DATASET_VERSION in PUBLIC_DATA_TEMPLATE diff --git a/tests/python/test_spatial_bench_profile.py b/tests/python/test_spatial_bench_profile.py index bd2aa22..615651d 100644 --- a/tests/python/test_spatial_bench_profile.py +++ b/tests/python/test_spatial_bench_profile.py @@ -217,7 +217,7 @@ def test_stage_table_drops_the_released_column_when_it_has_no_metrics(tmp_path): _results.write_profile_comparison({"branch": branch, "release": release}, out) text = out.read_text() - # Wall and peak still compare, so the released build stays in the top table + # Wall and peak still compare and the released build stays in the top table assert "-20.0%" in text and "-50.0%" in text assert "reports no engine metrics" in text assert "build prepared_polygons" in text