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 bbc739928ff7589c7de39b136cfec5436219a255 Author: tison <[email protected]> AuthorDate: Fri Aug 28 12:45:21 2026 +0800 refactor(req): align construction with builder conventions --- datasketches/src/req/mod.rs | 2 + datasketches/src/req/sketch.rs | 101 ++++++++++++++------- datasketches/src/req/union.rs | 58 ++++++++++-- tests-integration/tests/req_test/accuracy.rs | 2 +- tests-integration/tests/req_test/bounds.rs | 13 ++- tests-integration/tests/req_test/core.rs | 52 ++++++++--- tests-integration/tests/req_test/merge.rs | 21 ++--- tests-integration/tests/req_test/property.rs | 6 +- tests-integration/tests/req_test/query.rs | 12 +-- .../tests/req_test/sorted_view_api.rs | 8 +- tests-integration/tests/req_test/structure.rs | 8 +- tests-integration/tests/req_test/union.rs | 38 ++++++-- tests-integration/tests/serde_tests/req.rs | 4 +- 13 files changed, 225 insertions(+), 100 deletions(-) diff --git a/datasketches/src/req/mod.rs b/datasketches/src/req/mod.rs index 4baf163..cb5244e 100644 --- a/datasketches/src/req/mod.rs +++ b/datasketches/src/req/mod.rs @@ -41,8 +41,10 @@ fn nearest_even_section_size(value: f32) -> u32 { 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; pub use self::value::ReqValue; /// Default value of `k` if not specified. Roughly 1% relative error at 95% confidence. diff --git a/datasketches/src/req/sketch.rs b/datasketches/src/req/sketch.rs index ea6ddcd..df4e746 100644 --- a/datasketches/src/req/sketch.rs +++ b/datasketches/src/req/sketch.rs @@ -61,33 +61,26 @@ pub struct ReqSketch<T: ReqValue> { pub(super) max_item: Option<T>, } -#[derive(Debug, Clone, Copy)] -struct ValidatedReqConfig { - k: u16, - rank_accuracy: RankAccuracy, -} - -impl ValidatedReqConfig { - const DEFAULT: Self = Self { - k: DEFAULT_K, - rank_accuracy: RankAccuracy::HighRank, - }; - - fn validate(k: u16, rank_accuracy: RankAccuracy) -> Result<Self, 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}")); - } - Ok(Self { k, rank_accuracy }) +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}")); + } + Ok(()) } impl<T: ReqValue> ReqSketch<T> { - /// Creates a new sketch with default parameters (`k = 12`, `RankAccuracy::HighRank`). - pub fn new() -> Self { - Self::from_config(ValidatedReqConfig::DEFAULT) + /// Creates a new sketch with the given `k` and rank accuracy. + /// + /// The fallible version of this method is [`Self::try_new`]. + /// + /// # Panics + /// + /// Panics if `k` is odd or outside `[MIN_K, MAX_K]`. + pub fn new(k: u16, rank_accuracy: RankAccuracy) -> Self { + Self::try_new(k, rank_accuracy).unwrap_or_else(|error| panic!("{error}")) } /// Creates a new sketch with the given `k` and rank accuracy. @@ -96,9 +89,8 @@ impl<T: ReqValue> ReqSketch<T> { /// /// Returns an error if `k` is odd or outside `[MIN_K, MAX_K]`. pub fn try_new(k: u16, rank_accuracy: RankAccuracy) -> Result<Self, Error> { - let config = - ValidatedReqConfig::validate(k, rank_accuracy).map_err(Error::invalid_argument)?; - Ok(Self::from_config(config)) + validate_k(k).map_err(Error::invalid_argument)?; + Ok(Self::make(k, rank_accuracy)) } /// Returns the configured `k` parameter. @@ -584,7 +576,7 @@ impl<T: ReqValue> ReqSketch<T> { } else { RankAccuracy::LowRank }; - let config = ValidatedReqConfig::validate(k, rank_accuracy).map_err(Error::deserial)?; + validate_k(k).map_err(Error::deserial)?; if is_empty { if num_levels != 0 { @@ -597,7 +589,7 @@ impl<T: ReqValue> ReqSketch<T> { "empty REQ sketch must have 0 raw items, got {num_raw_items}" ))); } - return Ok(Self::from_config(config)); + return Ok(Self::make(k, rank_accuracy)); } if num_levels == 0 { @@ -720,7 +712,7 @@ impl<T: ReqValue> ReqSketch<T> { "REQ retained weighted count {weighted_count} does not match n {n}" ))); } - let mut sketch = Self::from_config(config); + let mut sketch = Self::make(k, rank_accuracy); sketch.n = n; sketch.min_item = min_item; sketch.max_item = max_item; @@ -732,15 +724,15 @@ impl<T: ReqValue> ReqSketch<T> { // --- Internal --- - fn from_config(config: ValidatedReqConfig) -> Self { + fn make(k: u16, rank_accuracy: RankAccuracy) -> Self { let mut sketch = Self { - k: config.k, - rank_accuracy: config.rank_accuracy, + k, + rank_accuracy, n: 0, max_nom_size: 0, num_retained: 0, compactors: Vec::new(), - promotion_buf: Vec::with_capacity(config.k as usize), + promotion_buf: Vec::with_capacity(k as usize), min_item: None, max_item: None, }; @@ -790,7 +782,48 @@ impl<T: ReqValue> ReqSketch<T> { impl<T: ReqValue> Default for ReqSketch<T> { fn default() -> Self { - Self::new() + 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) } } diff --git a/datasketches/src/req/union.rs b/datasketches/src/req/union.rs index 4ce49ae..e2c14ba 100644 --- a/datasketches/src/req/union.rs +++ b/datasketches/src/req/union.rs @@ -18,6 +18,7 @@ //! REQ union — combines REQ sketches into a single result. use crate::error::Error; +use crate::req::DEFAULT_K; use crate::req::RankAccuracy; use crate::req::sketch::ReqSketch; use crate::req::value::ReqValue; @@ -30,12 +31,24 @@ pub struct ReqUnion<T: ReqValue> { inner: ReqSketch<T>, } +/// Builder for [`ReqUnion`]. +#[derive(Debug, Clone)] +pub struct ReqUnionBuilder<T: ReqValue> { + k: u16, + rank_accuracy: RankAccuracy, + _marker: std::marker::PhantomData<T>, +} + impl<T: ReqValue> ReqUnion<T> { - /// Creates a new union with default `k = 12` and `RankAccuracy::HighRank`. - pub fn new() -> Self { - Self { - inner: ReqSketch::new(), - } + /// Creates a new union with the given `k` and rank accuracy. + /// + /// The fallible version of this method is [`Self::try_new`]. + /// + /// # Panics + /// + /// Panics if `k` is invalid (see [`ReqSketch::new`]). + pub fn new(k: u16, rank_accuracy: RankAccuracy) -> Self { + Self::try_new(k, rank_accuracy).unwrap_or_else(|error| panic!("{error}")) } /// Creates a new union with the given `k` and rank accuracy. @@ -89,6 +102,39 @@ impl<T: ReqValue> ReqUnion<T> { impl<T: ReqValue> Default for ReqUnion<T> { fn default() -> Self { - Self::new() + Self::new(DEFAULT_K, RankAccuracy::HighRank) + } +} + +impl<T: ReqValue> Default for ReqUnionBuilder<T> { + fn default() -> Self { + Self { + k: DEFAULT_K, + rank_accuracy: RankAccuracy::HighRank, + _marker: std::marker::PhantomData, + } + } +} + +impl<T: ReqValue> ReqUnionBuilder<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 union. + /// + /// # Errors + /// + /// Returns an error if `k` is invalid (see [`ReqUnion::try_new`]). + pub fn build(self) -> Result<ReqUnion<T>, Error> { + ReqUnion::try_new(self.k, self.rank_accuracy) } } diff --git a/tests-integration/tests/req_test/accuracy.rs b/tests-integration/tests/req_test/accuracy.rs index d8dc10b..8ca9fde 100644 --- a/tests-integration/tests/req_test/accuracy.rs +++ b/tests-integration/tests/req_test/accuracy.rs @@ -25,7 +25,7 @@ use googletest::prelude::le; #[test] fn rank_space_error_is_bounded() -> Result<(), Error> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); let n = 50_000; for i in 0..n { diff --git a/tests-integration/tests/req_test/bounds.rs b/tests-integration/tests/req_test/bounds.rs index a9268dc..ced8d01 100644 --- a/tests-integration/tests/req_test/bounds.rs +++ b/tests-integration/tests/req_test/bounds.rs @@ -29,8 +29,7 @@ use googletest::prelude::lt; #[test] fn bounds_are_nested_and_in_unit_interval() { - let mut sketch = - ReqSketch::try_new(12, RankAccuracy::HighRank).expect("construction should succeed"); + let mut sketch = ReqSketch::new(12, RankAccuracy::HighRank); for i in 0..50_000 { sketch.update(i as f64); @@ -61,7 +60,7 @@ fn bounds_are_nested_and_in_unit_interval() { #[test] fn theoretical_error_bounds_cover_uniform_quantiles() -> Result<(), Error> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); let n = 50_000; for i in 0..n { @@ -85,8 +84,8 @@ fn theoretical_error_bounds_cover_uniform_quantiles() -> Result<(), Error> { #[test] fn hra_and_lra_bounds_are_tighter_at_their_target_end() -> Result<(), Error> { for rank in [0.05, 0.25, 0.5, 0.75, 0.95] { - let mut hra = ReqSketch::try_new(12, RankAccuracy::HighRank)?; - let mut lra = ReqSketch::try_new(12, RankAccuracy::LowRank)?; + let mut hra = ReqSketch::new(12, RankAccuracy::HighRank); + let mut lra = ReqSketch::new(12, RankAccuracy::LowRank); for i in 0..10_000 { hra.update(i as f64); @@ -110,7 +109,7 @@ fn hra_and_lra_bounds_are_tighter_at_their_target_end() -> Result<(), Error> { #[test] fn exact_mode_bounds_are_tight() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..20 { sketch.update(i as f64); @@ -127,7 +126,7 @@ fn exact_mode_bounds_are_tight() { #[test] fn high_rank_accuracy_matches_tight_thresholds() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); let n = 50_000; for i in 0..n { diff --git a/tests-integration/tests/req_test/core.rs b/tests-integration/tests/req_test/core.rs index 271cc8a..1a07a70 100644 --- a/tests-integration/tests/req_test/core.rs +++ b/tests-integration/tests/req_test/core.rs @@ -21,6 +21,7 @@ use datasketches::error::Error; use datasketches::error::ErrorKind; use datasketches::req::RankAccuracy; use datasketches::req::ReqSketch; +use datasketches::req::ReqSketchBuilder; use datasketches::req::SearchCriteria; use googletest::assert_that; use googletest::prelude::all; @@ -35,7 +36,7 @@ use googletest::prelude::none; #[test] fn empty_sketch_has_default_state_and_rejects_queries() { - let sketch: ReqSketch<f32> = ReqSketch::new(); + let sketch: ReqSketch<f32> = ReqSketch::default(); assert_eq!(sketch.k(), 12); assert!(sketch.is_empty()); @@ -65,7 +66,7 @@ fn empty_sketch_has_default_state_and_rejects_queries() { #[test] fn single_value_hra_answers_exactly() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); sketch.update(1.0f32); assert!(!sketch.is_empty()); @@ -112,8 +113,10 @@ fn single_value_hra_answers_exactly() { #[test] fn single_value_lra_preserves_configuration() { - let mut sketch: ReqSketch<f32> = - ReqSketch::try_new(12, RankAccuracy::LowRank).expect("construction should succeed"); + let mut sketch = ReqSketchBuilder::<f32>::default() + .rank_accuracy(RankAccuracy::LowRank) + .build() + .expect("construction should succeed"); sketch.update(1.0f32); assert_eq!(sketch.rank_accuracy(), RankAccuracy::LowRank); @@ -125,7 +128,7 @@ fn single_value_lra_preserves_configuration() { #[test] fn repeated_values_respect_search_criteria() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for _ in 0..3 { sketch.update(1.0f32); } @@ -165,7 +168,7 @@ fn repeated_values_respect_search_criteria() { #[test] fn estimation_mode_compresses_and_keeps_min_max() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); let n = 100_000; for i in 0..n { @@ -196,7 +199,7 @@ fn estimation_mode_compresses_and_keeps_min_max() { #[test] fn nan_updates_are_silently_skipped_for_f64() { - let mut sketch: ReqSketch<f64> = ReqSketch::new(); + let mut sketch: ReqSketch<f64> = ReqSketch::default(); sketch.update(f64::NAN); sketch.update(f64::NAN); assert!(sketch.is_empty()); @@ -212,7 +215,7 @@ fn nan_updates_are_silently_skipped_for_f64() { #[test] fn nan_updates_are_silently_skipped_for_f32() { - let mut sketch: ReqSketch<f32> = ReqSketch::new(); + let mut sketch: ReqSketch<f32> = ReqSketch::default(); sketch.update(f32::NAN); assert!(sketch.is_empty()); assert_eq!(sketch.n(), 0); @@ -230,17 +233,17 @@ fn nan_updates_are_silently_skipped_for_f32() { #[test] fn small_edge_cases_answer_reasonably() -> Result<(), Error> { - let mut single = ReqSketch::new(); + let mut single = ReqSketch::default(); single.update(42.0); assert_eq!(single.quantile(0.5, SearchCriteria::Inclusive)?, 42.0); - let mut two_values = ReqSketch::new(); + let mut two_values = ReqSketch::default(); two_values.update(1.0); two_values.update(100.0); let median = two_values.quantile(0.5, SearchCriteria::Inclusive)?; assert_that!(median, all!(ge(1.0), le(100.0))); - let mut duplicates = ReqSketch::new(); + let mut duplicates = ReqSketch::default(); for _ in 0..100 { duplicates.update(42.0); } @@ -263,3 +266,30 @@ 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())); +} diff --git a/tests-integration/tests/req_test/merge.rs b/tests-integration/tests/req_test/merge.rs index aeb19d4..d4123bf 100644 --- a/tests-integration/tests/req_test/merge.rs +++ b/tests-integration/tests/req_test/merge.rs @@ -27,10 +27,8 @@ use googletest::prelude::near; #[test] fn merge_into_empty_preserves_source_distribution() { - let mut target: ReqSketch<f32> = - ReqSketch::try_new(40, RankAccuracy::HighRank).expect("construction should succeed"); - let mut source: ReqSketch<f32> = - ReqSketch::try_new(40, RankAccuracy::HighRank).expect("construction should succeed"); + let mut target: ReqSketch<f32> = ReqSketch::new(40, RankAccuracy::HighRank); + let mut source: ReqSketch<f32> = ReqSketch::new(40, RankAccuracy::HighRank); for i in 0..1000 { source.update(i as f32); @@ -61,10 +59,8 @@ fn merge_into_empty_preserves_source_distribution() { #[test] fn merge_two_ranges_preserves_distribution() { - let mut left: ReqSketch<f32> = - ReqSketch::try_new(100, RankAccuracy::HighRank).expect("construction should succeed"); - let mut right: ReqSketch<f32> = - ReqSketch::try_new(100, RankAccuracy::HighRank).expect("construction should succeed"); + let mut left: ReqSketch<f32> = ReqSketch::new(100, RankAccuracy::HighRank); + let mut right: ReqSketch<f32> = ReqSketch::new(100, RankAccuracy::HighRank); for i in 0..1000 { left.update(i as f32); @@ -98,9 +94,8 @@ fn merge_two_ranges_preserves_distribution() { #[test] fn merge_rejects_incompatible_accuracy_modes() { - let mut high_rank = ReqSketch::new(); - let low_rank: ReqSketch<f32> = - ReqSketch::try_new(12, RankAccuracy::LowRank).expect("construction should succeed"); + let mut high_rank = ReqSketch::default(); + let low_rank: ReqSketch<f32> = ReqSketch::new(12, RankAccuracy::LowRank); high_rank.update(1.0); assert_that!(high_rank.merge(&low_rank), err(anything())); @@ -108,10 +103,10 @@ fn merge_rejects_incompatible_accuracy_modes() { #[test] fn many_small_merges_preserve_count_bounds_and_median() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for batch in 0..100 { - let mut batch_sketch = ReqSketch::new(); + let mut batch_sketch = ReqSketch::default(); for i in 0..100 { batch_sketch.update((batch * 100 + i) as f64); } diff --git a/tests-integration/tests/req_test/property.rs b/tests-integration/tests/req_test/property.rs index cbaf130..b70952d 100644 --- a/tests-integration/tests/req_test/property.rs +++ b/tests-integration/tests/req_test/property.rs @@ -30,7 +30,7 @@ fn prop_quantile_rank_consistency() { return TestResult::discard(); } - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for value in values { sketch.update(value); } @@ -78,7 +78,7 @@ fn prop_sketch_bounds() { return TestResult::discard(); } - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for value in &values { sketch.update(*value); } @@ -119,7 +119,7 @@ fn prop_rank_monotonicity() { return TestResult::discard(); } - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for value in values { sketch.update(value); } diff --git a/tests-integration/tests/req_test/query.rs b/tests-integration/tests/req_test/query.rs index 38e9513..c07bff9 100644 --- a/tests-integration/tests/req_test/query.rs +++ b/tests-integration/tests/req_test/query.rs @@ -29,7 +29,7 @@ use googletest::prelude::near; #[test] fn exact_mode_rank_quantile_pmf_and_cdf_match_reference() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 1..=10 { sketch.update(i as f32); } @@ -94,7 +94,7 @@ fn exact_mode_rank_quantile_pmf_and_cdf_match_reference() { #[test] fn pmf_and_cdf_are_consistent() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..1000 { sketch.update(i as f64); } @@ -119,7 +119,7 @@ fn pmf_and_cdf_are_consistent() { #[test] fn rank_is_monotonic_and_bounded() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..10_000 { sketch.update(i as f64); } @@ -139,7 +139,7 @@ fn rank_is_monotonic_and_bounded() { #[test] fn quantiles_are_monotonic() -> Result<(), Error> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..10_000 { sketch.update(i as f64); } @@ -158,7 +158,7 @@ fn quantiles_are_monotonic() -> Result<(), Error> { #[test] fn rank_quantile_round_trip_is_consistent() -> Result<(), Error> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..10_000 { sketch.update(i as f64); } @@ -175,7 +175,7 @@ fn rank_quantile_round_trip_is_consistent() -> Result<(), Error> { #[test] fn search_criteria_rank_consistency() -> Result<(), Error> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..1000 { sketch.update(i as f64); } diff --git a/tests-integration/tests/req_test/sorted_view_api.rs b/tests-integration/tests/req_test/sorted_view_api.rs index f6eff2b..b5ed000 100644 --- a/tests-integration/tests/req_test/sorted_view_api.rs +++ b/tests-integration/tests/req_test/sorted_view_api.rs @@ -33,7 +33,7 @@ use googletest::prelude::lt; use googletest::prelude::near; fn populated_sketch(n: u64) -> ReqSketch<f64> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..n { sketch.update(i as f64); } @@ -85,7 +85,7 @@ fn sorted_view_is_an_owned_snapshot() { #[test] fn sorted_view_on_empty_sketch_is_an_empty_view() { - let sketch: ReqSketch<f64> = ReqSketch::new(); + let sketch: ReqSketch<f64> = ReqSketch::default(); let view = sketch.sorted_view(); assert!(view.is_empty()); assert_eq!(view.len(), 0); @@ -99,7 +99,7 @@ fn sorted_view_on_empty_sketch_is_an_empty_view() { #[test] fn empty_sketch_pmf_cdf_report_error() { - let sketch: ReqSketch<f64> = ReqSketch::new(); + let sketch: ReqSketch<f64> = ReqSketch::default(); assert_that!( sketch.pmf(&[1.0], SearchCriteria::Inclusive), err(anything()) @@ -136,7 +136,7 @@ fn nan_query_items_are_rejected() { #[test] fn error_precedence_empty_before_invalid_rank() { // On an empty sketch the emptiness is reported before the out-of-range rank. - let empty: ReqSketch<f64> = ReqSketch::new(); + let empty: ReqSketch<f64> = ReqSketch::default(); let empty_err = empty.quantile(2.0, SearchCriteria::Inclusive).unwrap_err(); assert_that!(empty_err.message(), contains_substring("empty")); diff --git a/tests-integration/tests/req_test/structure.rs b/tests-integration/tests/req_test/structure.rs index c5bd42d..cbe2838 100644 --- a/tests-integration/tests/req_test/structure.rs +++ b/tests-integration/tests/req_test/structure.rs @@ -28,7 +28,7 @@ use googletest::prelude::predicate; #[test] fn iterator_weights_sum_to_n_and_items_are_in_range() { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..1000 { sketch.update(i as f64); } @@ -46,7 +46,7 @@ fn iterator_weights_sum_to_n_and_items_are_in_range() { #[test] fn small_sketch_iterator_reports_unit_weights() { // Below the compaction threshold every retained item still has weight 1. - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..10 { sketch.update(i as f64); } @@ -59,7 +59,7 @@ fn small_sketch_iterator_reports_unit_weights() { #[test] fn empty_sketch_iterator_yields_nothing() { - let sketch: ReqSketch<i32> = ReqSketch::new(); + let sketch: ReqSketch<i32> = ReqSketch::default(); assert_eq!(sketch.iter().count(), 0); } @@ -67,7 +67,7 @@ fn empty_sketch_iterator_yields_nothing() { fn compaction_promotes_surviving_items_to_higher_weights() { // After enough updates to trigger compaction, surviving items are promoted up a // level at double weight, so the maximum item weight exceeds 1. - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in 0..100_000 { sketch.update(i as f64); } diff --git a/tests-integration/tests/req_test/union.rs b/tests-integration/tests/req_test/union.rs index 6a7b6ed..605d422 100644 --- a/tests-integration/tests/req_test/union.rs +++ b/tests-integration/tests/req_test/union.rs @@ -20,6 +20,7 @@ use datasketches::req::RankAccuracy; use datasketches::req::ReqSketch; use datasketches::req::ReqUnion; +use datasketches::req::ReqUnionBuilder; use datasketches::req::SearchCriteria; use googletest::assert_that; use googletest::prelude::anything; @@ -29,7 +30,7 @@ use googletest::prelude::near; #[test] fn union_equivalent_to_chained_merge() { let make = |range: std::ops::Range<u64>| -> ReqSketch<f64> { - let mut sketch = ReqSketch::new(); + let mut sketch = ReqSketch::default(); for i in range { sketch.update(i as f64); } @@ -39,13 +40,13 @@ fn union_equivalent_to_chained_merge() { let s2 = make(1000..2000); let s3 = make(2000..3000); - let mut union: ReqUnion<f64> = ReqUnion::new(); + let mut union: ReqUnion<f64> = ReqUnion::default(); union.merge(&s1).expect("merge should succeed"); union.merge(&s2).expect("merge should succeed"); union.merge(&s3).expect("merge should succeed"); let union_result = union.to_sketch(); - let mut via_merge: ReqSketch<f64> = ReqSketch::new(); + let mut via_merge: ReqSketch<f64> = ReqSketch::default(); via_merge.merge(&s1).expect("merge should succeed"); via_merge.merge(&s2).expect("merge should succeed"); via_merge.merge(&s3).expect("merge should succeed"); @@ -69,18 +70,18 @@ fn union_equivalent_to_chained_merge() { #[test] fn empty_union_returns_empty_sketch() { - let union: ReqUnion<f64> = ReqUnion::new(); + let union: ReqUnion<f64> = ReqUnion::default(); assert!(union.to_sketch().is_empty()); } #[test] fn reset_clears_union_state() { - let mut sketch: ReqSketch<f64> = ReqSketch::new(); + let mut sketch: ReqSketch<f64> = ReqSketch::default(); for i in 0..100 { sketch.update(i as f64); } - let mut union: ReqUnion<f64> = ReqUnion::new(); + let mut union: ReqUnion<f64> = ReqUnion::default(); union.merge(&sketch).expect("merge should succeed"); assert!(!union.is_empty()); @@ -95,11 +96,30 @@ fn try_new_validates_k() { err(anything()) ); assert!(ReqUnion::<f64>::try_new(12, RankAccuracy::HighRank).is_ok()); + assert_that!( + ReqUnionBuilder::<f64>::default().k(3).build(), + err(anything()) + ); +} + +#[test] +fn new_and_builder_preserve_configuration() { + let union = ReqUnion::<f64>::new(16, RankAccuracy::LowRank); + assert_eq!(union.k(), 16); + assert_eq!(union.rank_accuracy(), RankAccuracy::LowRank); + + let union = ReqUnionBuilder::<f64>::default() + .k(20) + .rank_accuracy(RankAccuracy::LowRank) + .build() + .expect("construction should succeed"); + assert_eq!(union.k(), 20); + assert_eq!(union.rank_accuracy(), RankAccuracy::LowRank); } #[test] fn empty_union_uses_default_configuration() { - let union: ReqUnion<f64> = ReqUnion::new(); + let union: ReqUnion<f64> = ReqUnion::default(); assert_eq!(union.k(), 12); assert_eq!(union.rank_accuracy(), RankAccuracy::HighRank); } @@ -107,12 +127,12 @@ fn empty_union_uses_default_configuration() { #[test] fn union_keeps_default_k_when_merging_mismatched_sketch() { // The union retains its own k even when fed a sketch built with a different k. - let mut other = ReqSketch::<f64>::try_new(16, RankAccuracy::HighRank).expect("valid k"); + let mut other = ReqSketch::<f64>::new(16, RankAccuracy::HighRank); for i in 0..50 { other.update(i as f64); } - let mut union: ReqUnion<f64> = ReqUnion::new(); + let mut union: ReqUnion<f64> = ReqUnion::default(); union.merge(&other).expect("merge should succeed"); let result = union.to_sketch(); diff --git a/tests-integration/tests/serde_tests/req.rs b/tests-integration/tests/serde_tests/req.rs index c61d8cb..7a38af5 100644 --- a/tests-integration/tests/serde_tests/req.rs +++ b/tests-integration/tests/serde_tests/req.rs @@ -232,8 +232,8 @@ fn deserialize_truncated_raw_items() { #[test] fn merge_preserves_order_across_serde_round_trip() { - let mut high = ReqSketch::<f64>::new(); - let mut low = ReqSketch::<f64>::new(); + let mut high = ReqSketch::<f64>::default(); + let mut low = ReqSketch::<f64>::default(); for value in 1000..=1072 { high.update(value as f64); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
