fix: distinguish empty sketch queries from invalid arguments

CHANGELOG.md

@@ -9,11 +9,13 @@ All significant changes to this project will be documented in this file.
 * `BloomFilter::invert` is removed; use `BloomFilter::difference` for approximate A-not-B. The result excludes items in the right filter, but hash collisions can also remove items unique to the left filter.
 * `FrequentItemsSketch::is_empty` now returns `false` when the stream weight is nonzero, even if no items are retained. Use `num_active_items() == 0` to test for zero retained items.
 * `ReqSketch` queries now use `datasketches::common::SearchCriteria` instead of `datasketches::req::SearchCriteria`. `SearchCriteria` no longer implements `Default`; explicitly choose `Inclusive` or `Exclusive`.
+* `ReqSketch` and `req::SortedView` queries now distinguish empty sketches from invalid arguments. `rank` returns `Option<f64>`; `quantile`, `quantiles`, `cdf`, and `pmf`, where available, return `Result<Option<_>, Error>`. Handle `None` for valid queries on an empty sketch, and propagate or handle `InvalidArgument` for invalid ranks or split points, which are checked before emptiness.
+* `TDigest` and `TDigestMut` rank, quantile, CDF, and PMF queries now return `Result<Option<_>, Error>`. Invalid arguments return `InvalidArgument` instead of panicking, including on empty digests. Propagate or handle the `Result` before handling `None` for an empty digest; `min_value` and `max_value` still return `Option<f64>`.
 
 ### New features
 
 * Add `HllSketch::composite_estimate` and `HllUnion::composite_estimate` to query cardinality without relying on HIP update history, matching Java's `getCompositeEstimate()`.
-* `KllSketch` is now available behind the `kll` feature, with rank, quantile, PMF, and CDF queries, merging, serialization, custom ordered item types, and a `KllFloat` adapter for non-NaN floating-point values.
+* `KllSketch` is now available behind the `kll` feature, with rank, quantile, PMF, and CDF queries, merging, serialization, custom ordered item types, and a `KllFloat` adapter for non-NaN floating-point values. Queries distinguish an empty sketch (`None`) from invalid arguments (`InvalidArgument`).
 * Add `TDigestMut::quantiles` and `TDigest::quantiles` for querying several ranks in one centroid scan while preserving input order.
 * Add `FromIterator<TDigestMut>` as an alternative to incremental `TDigestMut::merge` calls for combining owned partial sketches in a single compression pass.
 

CONTRIBUTING.md

@@ -80,6 +80,13 @@ cargo bench --package benchmarks --bench benchmarks -- cpc::serde
 - Describe types with noun phrases and API behavior with third-person present-tense verbs such as `Creates`, `Updates`, and `Returns`.
 - End summary sentences with punctuation, and format Rust identifiers, literals, and numeric ranges as inline code.
 - Put contract sections and compatibility notes before examples. When applicable, order sections as `# Errors`, `# Panics`, and `# Examples`. Include only sections that describe an actual contract.
+- Document return values and error conditions on the relevant methods rather than repeating them in crate or module overviews.
+- Keep imports visible in doctests. A trailing hidden `Ok(...)` may be used to support `?`.
+
+## Query semantics
+
+- Validate query arguments before handling an empty sketch. Invalid arguments return `InvalidArgument` regardless of whether the sketch is empty.
+- Return a meaningful result when one exists, such as zero, `false`, or an empty collection. Otherwise, use `Option` for an absent result, combined with `Result` when argument validation can fail.
 
 ## Visibility
 

benchmarks/tdigest/aggregation.rs

@@ -127,4 +127,5 @@ fn final_quantile(mut merged: TDigestMut) -> Option<f64> {
     TDigestMut::deserialize(black_box(&bytes))
         .unwrap()
         .quantile(black_box(0.9))
+        .unwrap()
 }

benchmarks/tdigest/compress.rs

@@ -31,22 +31,22 @@ fn initial_buffer(bencher: Bencher) {
     bencher
         .counter(ItemsCount::new(values.len()))
         .with_inputs(|| digest.clone())
-        .bench_local_values(|mut digest| black_box(digest.rank(0.5)));
+        .bench_local_values(|mut digest| black_box(digest.rank(0.5).unwrap()));
 }
 
 #[divan::bench]
 fn unmerged_tail(bencher: Bencher) {
     let values = values(3_280);
     let mut digest = build_mut_digest(&values[..1_640]);
-    black_box(digest.rank(0.5));
+    black_box(digest.rank(0.5).unwrap());
     for &value in &values[1_640..] {
         digest.update(value);
     }
 
     bencher
         .counter(ItemsCount::new(1_640_usize))
         .with_inputs(|| digest.clone())
-        .bench_local_values(|mut digest| black_box(digest.rank(0.5)));
+        .bench_local_values(|mut digest| black_box(digest.rank(0.5).unwrap()));
 }
 
 #[divan::bench]

benchmarks/tdigest/merge.rs

@@ -34,8 +34,8 @@ fn merge(bencher: Bencher) {
     let values = values(200_000);
     let mut left = build_mut_digest(&values[..100_000]);
     let mut right = build_mut_digest(&values[100_000..]);
-    black_box(left.rank(0.5));
-    black_box(right.rank(0.5));
+    black_box(left.rank(0.5).unwrap());
+    black_box(right.rank(0.5).unwrap());
 
     bencher
         .counter(ItemsCount::new(values.len()))
@@ -65,7 +65,7 @@ fn small_partials(bencher: Bencher) {
     let partials = partial_digests(64, SMALL_ROWS_PER_PARTIAL)
         .into_iter()
         .map(|mut digest| {
-            black_box(digest.quantile(0.5));
+            black_box(digest.quantile(0.5).unwrap());
             digest
         })
         .collect::<Vec<_>>();
@@ -86,7 +86,7 @@ fn partials(bencher: Bencher) {
     let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL)
         .into_iter()
         .map(|mut digest| {
-            black_box(digest.quantile(0.5));
+            black_box(digest.quantile(0.5).unwrap());
             digest
         })
         .collect::<Vec<_>>();
@@ -107,7 +107,7 @@ fn partials_from_iter(bencher: Bencher) {
     let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL)
         .into_iter()
         .map(|mut digest| {
-            black_box(digest.quantile(0.5));
+            black_box(digest.quantile(0.5).unwrap());
             digest
         })
         .collect::<Vec<_>>();
@@ -125,7 +125,7 @@ fn partials_from_iter_cloned(bencher: Bencher) {
     let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL)
         .into_iter()
         .map(|mut digest| {
-            black_box(digest.quantile(0.5));
+            black_box(digest.quantile(0.5).unwrap());
             digest
         })
         .collect::<Vec<_>>();
@@ -141,7 +141,7 @@ fn partials_from_iter_finalize(bencher: Bencher, additional_rows: usize) {
     let partials = partial_digests_with(DEFAULT_DIGEST_K, 64, ROWS_PER_PARTIAL)
         .into_iter()
         .map(|mut digest| {
-            black_box(digest.quantile(0.5));
+            black_box(digest.quantile(0.5).unwrap());
             digest
         })
         .collect::<Vec<_>>();
@@ -188,7 +188,7 @@ fn serialized_partials(bencher: Bencher, rows_per_partial: usize) {
                 let partial = TDigestMut::deserialize(partial).unwrap();
                 merged.merge(&partial);
             }
-            black_box(merged.quantile(0.5))
+            black_box(merged.quantile(0.5).unwrap())
         });
 }
 
@@ -208,7 +208,7 @@ fn serialized_overlapping_partials(bencher: Bencher) {
                 let partial = TDigestMut::deserialize(partial).unwrap();
                 merged.merge(&partial);
             }
-            black_box(merged.quantile(0.5))
+            black_box(merged.quantile(0.5).unwrap())
         });
 }
 
@@ -224,7 +224,7 @@ fn serialized_partials_from_iter(bencher: Bencher, overlapping: bool) {
                 .map(|partial| TDigestMut::deserialize(partial))
                 .collect::<Result<TDigestMut, _>>()
                 .unwrap();
-            black_box(merged.quantile(0.5))
+            black_box(merged.quantile(0.5).unwrap())
         });
 }
 
@@ -244,7 +244,7 @@ fn serialized_partials_in_batches(bencher: Bencher, overlapping: bool) {
                     .unwrap();
                 merged.merge(&batch);
             }
-            black_box(merged.quantile(0.5))
+            black_box(merged.quantile(0.5).unwrap())
         });
 }
 

benchmarks/tdigest/query.rs

@@ -29,22 +29,22 @@ use super::support::prepared_digest;
 fn rank(bencher: Bencher) {
     let digest = prepared_digest();
 
-    bencher.bench_local(|| black_box(&digest).rank(black_box(0.531_25)));
+    bencher.bench_local(|| black_box(&digest).rank(black_box(0.531_25)).unwrap());
 }
 
 #[divan::bench(args = [0.0, 3.0, 3.5, 7.0])]
 fn rank_repeated_means(bencher: Bencher, value: f64) {
     let values = (0..100_000).map(|i| (i % 8) as f64).collect::<Vec<_>>();
     let digest = build_digest(&values);
 
-    bencher.bench_local(|| black_box(&digest).rank(black_box(value)));
+    bencher.bench_local(|| black_box(&digest).rank(black_box(value)).unwrap());
 }
 
 #[divan::bench]
 fn quantile(bencher: Bencher) {
     let digest = prepared_digest();
 
-    bencher.bench_local(|| black_box(&digest).quantile(black_box(0.531_25)));
+    bencher.bench_local(|| black_box(&digest).quantile(black_box(0.531_25)).unwrap());
 }
 
 #[divan::bench]
@@ -54,7 +54,7 @@ fn cdf_100(bencher: Bencher) {
 
     bencher
         .counter(ItemsCount::new(split_points.len()))
-        .bench_local(|| black_box(&digest).cdf(black_box(&split_points)));
+        .bench_local(|| black_box(&digest).cdf(black_box(&split_points)).unwrap());
 }
 
 #[divan::bench(consts = [true, false], args = [2, 6, 100])]
@@ -64,7 +64,7 @@ fn quantiles_sequential<const SORTED: bool>(bencher: Bencher, num_ranks: usize)
 
     bencher.counter(ItemsCount::new(num_ranks)).bench_local(|| {
         for &rank in black_box(&ranks) {
-            black_box(black_box(&digest).quantile(black_box(rank)));
+            black_box(black_box(&digest).quantile(black_box(rank)).unwrap());
         }
     });
 }
@@ -76,7 +76,7 @@ fn quantiles_batch<const SORTED: bool>(bencher: Bencher, num_ranks: usize) {
 
     bencher
         .counter(ItemsCount::new(num_ranks))
-        .bench_local(|| black_box(&digest).quantiles(black_box(&ranks)));
+        .bench_local(|| black_box(&digest).quantiles(black_box(&ranks)).unwrap());
 }
 
 fn query_ranks(num_ranks: usize, sorted: bool) -> Vec<f64> {

datasketches/src/kll/mod.rs

@@ -31,13 +31,17 @@
 //! # Usage
 //!
 //! ```rust
-//! # use datasketches::common::SearchCriteria;
-//! # use datasketches::kll::KllSketch;
-//! let mut sketch = KllSketch::<i64>::new(200).unwrap();
+//! use datasketches::common::SearchCriteria;
+//! use datasketches::kll::KllSketch;
+//!
+//! let mut sketch = KllSketch::<i64>::new(200)?;
 //! sketch.update(1);
 //! sketch.update(2);
-//! let q = sketch.quantile(0.5, SearchCriteria::Inclusive).unwrap();
+//! let Some(q) = sketch.quantile(0.5, SearchCriteria::Inclusive)? else {
+//!     return Ok(()); // No samples to query.
+//! };
 //! assert!((1..=2).contains(&q));
+//! # Ok::<(), datasketches::error::Error>(())
 //! ```
 
 mod capacity;

datasketches/src/kll/sketch.rs

@@ -79,7 +79,8 @@ impl<T: Clone + Ord> KllSketch<T> {
     /// # Examples
     ///
     /// ```
-    /// # use datasketches::kll::KllSketch;
+    /// use datasketches::kll::KllSketch;
+    ///
     /// let sketch = KllSketch::<i64>::new(200).unwrap();
     /// assert_eq!(sketch.k(), 200);
     /// ```
@@ -123,11 +124,15 @@ impl<T: Clone + Ord> KllSketch<T> {
     }
 
     /// Returns the minimum item seen by the sketch.
+    ///
+    /// Returns `None` if the sketch is empty.
     pub fn min_item(&self) -> Option<&T> {
         self.min_item.as_ref()
     }
 
     /// Returns the maximum item seen by the sketch.
+    ///
+    /// Returns `None` if the sketch is empty.
     pub fn max_item(&self) -> Option<&T> {
         self.max_item.as_ref()
     }
@@ -202,12 +207,10 @@ impl<T: Clone + Ord> KllSketch<T> {
 
     /// Returns the normalized rank of the given item.
     ///
-    /// # Errors
-    ///
-    /// Returns an error if the sketch is empty.
-    pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Result<f64, Error> {
+    /// Returns `None` if the sketch is empty.
+    pub fn rank(&self, item: &T, criteria: SearchCriteria) -> Option<f64> {
         if self.is_empty() {
-            return Err(Error::invalid_argument("cannot query an empty sketch"));
+            return None;
         }
         let inclusive = criteria == SearchCriteria::Inclusive;
         let mut weight = 0u64;
@@ -222,70 +225,87 @@ impl<T: Clone + Ord> KllSketch<T> {
                 .count() as u64;
             weight += count << level;
         }
-        Ok(weight as f64 / self.n as f64)
+        Some(weight as f64 / self.n as f64)
     }
 
     /// Returns the quantile for the given normalized rank.
     ///
-    /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream.
+    /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns
+    /// `Ok(None)` if the sketch is empty.
     ///
     /// # Errors
     ///
-    /// Returns an error if the sketch is empty or `rank` is outside `[0.0, 1.0]`.
-    pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result<T, Error> {
-        if self.is_empty() {
-            return Err(Error::invalid_argument("cannot query an empty sketch"));
-        }
+    /// Returns `InvalidArgument` if `rank` is NaN or outside `[0.0, 1.0]`, even if the sketch is
+    /// empty.
+    pub fn quantile(&self, rank: f64, criteria: SearchCriteria) -> Result<Option<T>, Error> {
         if !(0.0..=1.0).contains(&rank) {
             return Err(Error::invalid_argument(format!(
                 "rank must be in [0.0, 1.0], got {rank}"
             )));
         }
+        if self.is_empty() {
+            return Ok(None);
+        }
         if rank == 0.0 {
-            return Ok(self.min_item.as_ref().unwrap().clone());
+            return Ok(Some(self.min_item.as_ref().unwrap().clone()));
         }
         if rank == 1.0 {
-            return Ok(self.max_item.as_ref().unwrap().clone());
+            return Ok(Some(self.max_item.as_ref().unwrap().clone()));
         }
         self.sorted_view().quantile(rank, criteria)
     }
 
     /// Returns approximate quantiles for the given normalized ranks.
     ///
-    /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream.
+    /// Ranks `0.0` and `1.0` return the exact minimum and maximum items of the stream. Returns
+    /// `Ok(None)` if the sketch is empty. Otherwise, an empty `ranks` slice returns
+    /// `Ok(Some(vec![]))`.
     ///
     /// The sorted view is built once for the whole batch.
     ///
     /// # Errors
     ///
-    /// Returns an error if the sketch is empty or any rank is outside `[0.0, 1.0]`.
-    pub fn quantiles(&self, ranks: &[f64], criteria: SearchCriteria) -> Result<Vec<T>, Error> {
+    /// Returns `InvalidArgument` if any rank is NaN or outside `[0.0, 1.0]`, even if the sketch is
+    /// empty.
+    pub fn quantiles(
+        &self,
+        ranks: &[f64],
+        criteria: SearchCriteria,
+    ) -> Result<Option<Vec<T>>, Error> {
         self.sorted_view().quantiles(ranks, criteria)
     }
 
     /// Returns the approximate CDF for the given split points.
     ///
+    /// Returns `Ok(None)` if the sketch is empty. Otherwise, an empty `split_points` slice returns
+    /// `Ok(Some(vec![1.0]))`.
+    ///
     /// # Errors
     ///
-    /// Returns an error if the sketch is empty or the split points are not unique and strictly
-    /// increasing.
-    pub fn cdf(&self, split_points: &[T], criteria: SearchCriteria) -> Result<Vec<f64>, Error> {
-        if self.is_empty() {
-            return Err(Error::invalid_argument("cannot query an empty sketch"));
-        }
+    /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the
+    /// sketch is empty.
+    pub fn cdf(
+        &self,
+        split_points: &[T],
+        criteria: SearchCriteria,
+    ) -> Result<Option<Vec<f64>>, Error> {
         self.sorted_view().cdf(split_points, criteria)
     }
 
     /// Returns the approximate PMF for the given split points.
     ///
+    /// Returns `Ok(None)` if the sketch is empty. Otherwise, an empty `split_points` slice returns
+    /// `Ok(Some(vec![1.0]))`.
+    ///
     /// # Errors
     ///
-    /// Returns an error if the sketch is empty or the split points are not unique and strictly
-    /// increasing.
-    pub fn pmf(&self, split_points: &[T], criteria: SearchCriteria) -> Result<Vec<f64>, Error> {
-        if self.is_empty() {
-            return Err(Error::invalid_argument("cannot query an empty sketch"));
-        }
+    /// Returns `InvalidArgument` if the split points are not strictly increasing, even if the
+    /// sketch is empty.
+    pub fn pmf(
+        &self,
+        split_points: &[T],
+        criteria: SearchCriteria,
+    ) -> Result<Option<Vec<f64>>, Error> {
         self.sorted_view().pmf(split_points, criteria)
     }