This is an automated email from the ASF dual-hosted git repository. tisonkun pushed a commit to branch pr/223 in repository https://gitbox.apache.org/repos/asf/datasketches-rust.git
commit 5be79f16537c206a1d8f1063aa5fbaa9a8502d8f Author: tison <[email protected]> AuthorDate: Fri Aug 28 14:34:59 2026 +0800 fixup Signed-off-by: tison <[email protected]> --- datasketches/src/req/compactor.rs | 2 +- datasketches/src/req/mod.rs | 17 ++-- datasketches/src/req/sketch.rs | 137 ++++++++++--------------------- datasketches/src/req/sorted_view.rs | 4 +- datasketches/src/tdigest/sketch.rs | 1 - tests-integration/tests/req_test/core.rs | 28 +------ 6 files changed, 56 insertions(+), 133 deletions(-) diff --git a/datasketches/src/req/compactor.rs b/datasketches/src/req/compactor.rs index f614eec..da79181 100644 --- a/datasketches/src/req/compactor.rs +++ b/datasketches/src/req/compactor.rs @@ -480,7 +480,7 @@ where items, is_sorted, state, - scratch_buffer: Vec::new(), + scratch_buffer: vec![], section_size: nearest_even_section_size(section_size_raw), num_sections, lg_weight, diff --git a/datasketches/src/req/mod.rs b/datasketches/src/req/mod.rs index cb5244e..a0a67d7 100644 --- a/datasketches/src/req/mod.rs +++ b/datasketches/src/req/mod.rs @@ -31,17 +31,8 @@ mod sorted_view; mod union; mod value; -/// Number of sections in a newly created compactor. The section count and size -/// determine its capacity and compaction range; the count doubles as its state grows. -const INITIAL_SECTIONS_PER_COMPACTOR: u8 = 3; - -fn nearest_even_section_size(value: f32) -> u32 { - ((value / 2.0).round() as u32) << 1 -} - pub use self::iter::ReqSketchIterator; pub use self::sketch::ReqSketch; -pub use self::sketch::ReqSketchBuilder; pub use self::sorted_view::SortedView; pub use self::union::ReqUnion; pub use self::union::ReqUnionBuilder; @@ -73,3 +64,11 @@ pub enum SearchCriteria { /// Exclude the weight of the search item from the result. Exclusive, } + +/// Number of sections in a newly created compactor. The section count and size +/// determine its capacity and compaction range; the count doubles as its state grows. +const INITIAL_SECTIONS_PER_COMPACTOR: u8 = 3; + +fn nearest_even_section_size(value: f32) -> u32 { + ((value / 2.0).round() as u32) << 1 +} diff --git a/datasketches/src/req/sketch.rs b/datasketches/src/req/sketch.rs index df4e746..ab1422e 100644 --- a/datasketches/src/req/sketch.rs +++ b/datasketches/src/req/sketch.rs @@ -17,8 +17,6 @@ //! REQ sketch — generic over `T: ReqValue`. -use std::fmt; - use crate::codec::SketchBytes; use crate::codec::SketchSlice; use crate::codec::assert::insufficient_data; @@ -50,25 +48,21 @@ use crate::req::value::ReqValue; /// See the [module-level documentation](super) for background. #[derive(Debug, Clone)] pub struct ReqSketch<T: ReqValue> { - pub(super) k: u16, - pub(super) rank_accuracy: RankAccuracy, - pub(super) n: u64, - pub(super) max_nom_size: u32, - pub(super) num_retained: u32, - pub(super) compactors: Vec<Compactor<T>>, - pub(super) promotion_buf: Vec<T>, - pub(super) min_item: Option<T>, - pub(super) max_item: Option<T>, + k: u16, + rank_accuracy: RankAccuracy, + n: u64, + max_nom_size: u32, + num_retained: u32, + compactors: Vec<Compactor<T>>, + promotion_buf: Vec<T>, + min_item: Option<T>, + max_item: Option<T>, } -fn validate_k(k: u16) -> Result<(), String> { - if !(MIN_K..=MAX_K).contains(&k) { - return Err(format!("k must be in [{MIN_K}, {MAX_K}], got {k}")); - } - if k % 2 != 0 { - return Err(format!("k must be even, got {k}")); +impl<T: ReqValue> Default for ReqSketch<T> { + fn default() -> Self { + Self::new(DEFAULT_K, RankAccuracy::HighRank) } - Ok(()) } impl<T: ReqValue> ReqSketch<T> { @@ -78,18 +72,25 @@ impl<T: ReqValue> ReqSketch<T> { /// /// # Panics /// - /// Panics if `k` is odd or outside `[MIN_K, MAX_K]`. + /// Panics if `k` is odd or outside `[4, 1024]`. pub fn new(k: u16, rank_accuracy: RankAccuracy) -> Self { - Self::try_new(k, rank_accuracy).unwrap_or_else(|error| panic!("{error}")) + Self::make(k, rank_accuracy) } /// Creates a new sketch with the given `k` and rank accuracy. /// /// # Errors /// - /// Returns an error if `k` is odd or outside `[MIN_K, MAX_K]`. + /// Returns an error if `k` is odd or outside `[4, 1024]`. pub fn try_new(k: u16, rank_accuracy: RankAccuracy) -> Result<Self, Error> { - validate_k(k).map_err(Error::invalid_argument)?; + if !(MIN_K..=MAX_K).contains(&k) { + return Err(Error::invalid_argument(format!( + "k must be in [{MIN_K}, {MAX_K}], got {k}" + ))); + } + if k % 2 != 0 { + return Err(Error::invalid_argument(format!("k must be even, got {k}"))); + } Ok(Self::make(k, rank_accuracy)) } @@ -576,7 +577,15 @@ impl<T: ReqValue> ReqSketch<T> { } else { RankAccuracy::LowRank }; - validate_k(k).map_err(Error::deserial)?; + + if !(MIN_K..=MAX_K).contains(&k) { + return Err(Error::deserial(format!( + "k must be in [{MIN_K}, {MAX_K}], got {k}" + ))); + } + if k % 2 != 0 { + return Err(Error::deserial(format!("k must be even, got {k}"))); + } if is_empty { if num_levels != 0 { @@ -722,16 +731,20 @@ impl<T: ReqValue> ReqSketch<T> { Ok(sketch) } - // --- Internal --- - fn make(k: u16, rank_accuracy: RankAccuracy) -> Self { + assert!( + (MIN_K..=MAX_K).contains(&k), + "k must be in [{MIN_K}, {MAX_K}], got {k}" + ); + assert_eq!(k % 2, 0, "k must be even, got {k}"); + let mut sketch = Self { k, rank_accuracy, n: 0, max_nom_size: 0, num_retained: 0, - compactors: Vec::new(), + compactors: vec![], promotion_buf: Vec::with_capacity(k as usize), min_item: None, max_item: None, @@ -743,14 +756,14 @@ impl<T: ReqValue> ReqSketch<T> { sketch } - pub(super) fn grow(&mut self) { + fn grow(&mut self) { let level = self.compactors.len() as u8; let compactor = Compactor::new(level, self.k, self.rank_accuracy); self.compactors.push(compactor); self.update_max_nom_size(); } - pub(super) fn compress(&mut self) { + fn compress(&mut self) { for h in 0..self.compactors.len() { if self.compactors[h].num_items() >= self.compactors[h].nominal_capacity() { if h == 0 { @@ -771,75 +784,11 @@ impl<T: ReqValue> ReqSketch<T> { } } - pub(super) fn update_max_nom_size(&mut self) { + fn update_max_nom_size(&mut self) { self.max_nom_size = self.compactors.iter().map(|c| c.nominal_capacity()).sum(); } - pub(super) fn update_num_retained(&mut self) { + fn update_num_retained(&mut self) { self.num_retained = self.compactors.iter().map(|c| c.num_items()).sum(); } } - -impl<T: ReqValue> Default for ReqSketch<T> { - fn default() -> Self { - Self::new(DEFAULT_K, RankAccuracy::HighRank) - } -} - -/// Builder for [`ReqSketch`]. -#[derive(Debug, Clone)] -pub struct ReqSketchBuilder<T: ReqValue> { - k: u16, - rank_accuracy: RankAccuracy, - _marker: std::marker::PhantomData<T>, -} - -impl<T: ReqValue> Default for ReqSketchBuilder<T> { - fn default() -> Self { - Self { - k: DEFAULT_K, - rank_accuracy: RankAccuracy::HighRank, - _marker: std::marker::PhantomData, - } - } -} - -impl<T: ReqValue> ReqSketchBuilder<T> { - /// Sets the `k` parameter. - pub fn k(mut self, k: u16) -> Self { - self.k = k; - self - } - - /// Sets the rank accuracy. - pub fn rank_accuracy(mut self, rank_accuracy: RankAccuracy) -> Self { - self.rank_accuracy = rank_accuracy; - self - } - - /// Builds the sketch. - /// - /// # Errors - /// - /// Returns an error if `k` is odd or outside `[MIN_K, MAX_K]`. - pub fn build(self) -> Result<ReqSketch<T>, Error> { - ReqSketch::try_new(self.k, self.rank_accuracy) - } -} - -impl<T: ReqValue + fmt::Display> fmt::Display for ReqSketch<T> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - writeln!(f, "REQ Sketch Summary:")?; - writeln!(f, " k : {}", self.k)?; - writeln!(f, " rank accuracy : {:?}", self.rank_accuracy)?; - writeln!(f, " n : {}", self.n)?; - writeln!(f, " num retained : {}", self.num_retained)?; - writeln!(f, " num levels : {}", self.compactors.len())?; - writeln!(f, " estimation mode : {}", self.is_estimation_mode())?; - if let (Some(min), Some(max)) = (&self.min_item, &self.max_item) { - writeln!(f, " min item : {min}")?; - writeln!(f, " max item : {max}")?; - } - Ok(()) - } -} diff --git a/datasketches/src/req/sorted_view.rs b/datasketches/src/req/sorted_view.rs index 06ddcc0..a8c1d98 100644 --- a/datasketches/src/req/sorted_view.rs +++ b/datasketches/src/req/sorted_view.rs @@ -53,8 +53,8 @@ where pub(super) fn new(mut weighted_items: Vec<(T, u64)>) -> Self { if weighted_items.is_empty() { return Self { - items: Vec::new(), - cumulative_weights: Vec::new(), + items: vec![], + cumulative_weights: vec![], total_weight: 0, }; } diff --git a/datasketches/src/tdigest/sketch.rs b/datasketches/src/tdigest/sketch.rs index 9acb558..cfc4105 100644 --- a/datasketches/src/tdigest/sketch.rs +++ b/datasketches/src/tdigest/sketch.rs @@ -247,7 +247,6 @@ impl TDigestMut { )) } - // for deserialization fn make( k: u16, reverse_merge: bool, diff --git a/tests-integration/tests/req_test/core.rs b/tests-integration/tests/req_test/core.rs index 1a07a70..8e5804c 100644 --- a/tests-integration/tests/req_test/core.rs +++ b/tests-integration/tests/req_test/core.rs @@ -19,9 +19,9 @@ use datasketches::error::Error; use datasketches::error::ErrorKind; +use datasketches::req::DEFAULT_K; use datasketches::req::RankAccuracy; use datasketches::req::ReqSketch; -use datasketches::req::ReqSketchBuilder; use datasketches::req::SearchCriteria; use googletest::assert_that; use googletest::prelude::all; @@ -113,10 +113,7 @@ fn single_value_hra_answers_exactly() { #[test] fn single_value_lra_preserves_configuration() { - let mut sketch = ReqSketchBuilder::<f32>::default() - .rank_accuracy(RankAccuracy::LowRank) - .build() - .expect("construction should succeed"); + let mut sketch = ReqSketch::<f32>::new(DEFAULT_K, RankAccuracy::LowRank); sketch.update(1.0f32); assert_eq!(sketch.rank_accuracy(), RankAccuracy::LowRank); @@ -267,29 +264,8 @@ fn try_new_validates_k() { assert!(ReqSketch::<f64>::try_new(12, RankAccuracy::HighRank).is_ok()); } -#[test] -fn new_and_builder_preserve_configuration() { - let sketch = ReqSketch::<f64>::new(16, RankAccuracy::LowRank); - assert_eq!(sketch.k(), 16); - assert_eq!(sketch.rank_accuracy(), RankAccuracy::LowRank); - - let sketch = ReqSketchBuilder::<f64>::default() - .k(20) - .rank_accuracy(RankAccuracy::LowRank) - .build() - .expect("construction should succeed"); - assert_eq!(sketch.k(), 20); - assert_eq!(sketch.rank_accuracy(), RankAccuracy::LowRank); -} - #[test] #[should_panic(expected = "k must be even")] fn new_panics_on_invalid_k() { let _ = ReqSketch::<f64>::new(5, RankAccuracy::HighRank); } - -#[test] -fn builder_validates_k_at_build() { - let builder = ReqSketchBuilder::<f64>::default().k(5); - assert_that!(builder.build(), err(anything())); -} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
