notfilippo commented on code in PR #58: URL: https://github.com/apache/datasketches-rust/pull/58#discussion_r2657287763
########## datasketches/src/frequencies/mod.rs: ########## @@ -23,6 +23,29 @@ //! //! For background, see the Java documentation: //! <https://apache.github.io/datasketches-java/9.0.0/org/apache/datasketches/frequencies/FrequentItemsSketch.html> +//! +//! # Usage +//! +//! ```rust +//! # use datasketches::frequencies::ErrorType; +//! # use datasketches::frequencies::FrequentItemsSketch; +//! let mut sketch = FrequentItemsSketch::<i64>::new(64); +//! sketch.update_with_count(1, 3); +//! sketch.update(2); +//! let rows = sketch.frequent_items(ErrorType::NoFalseNegatives); +//! assert!(rows.iter().any(|row| *row.item() == 1)); +//! ``` +//! +//! # Serialization +//! +//! ```rust +//! # use datasketches::frequencies::FrequentItemsSketch; +//! let mut sketch = FrequentItemsSketch::<i64>::new(64); +//! sketch.update_with_count(42, 2); +//! +//! let bytes = sketch.serialize(); +//! let _decoded = FrequentItemsSketch::<i64>::deserialize(&bytes).expect("valid bytes"); Review Comment: Feel free to remove the `_` prefix, IIUC the compiler should not complain about that. ########## datasketches/src/countmin/sketch.rs: ########## @@ -195,6 +252,16 @@ impl CountMinSketch { } /// Serializes this sketch into the DataSketches Count-Min format. + /// + /// # Examples + /// + /// ``` + /// # use datasketches::countmin::CountMinSketch; + /// # let mut sketch = CountMinSketch::new(4, 128); + /// # sketch.update("apple"); + /// let bytes = sketch.serialize(); + /// let _decoded = CountMinSketch::deserialize(&bytes).expect("valid bytes"); Review Comment: As per [Rust common message conventions](https://doc.rust-lang.org/std/error/index.html#common-message-styles), expect should describe verbosely what we would expect from the operation (usually with a "should" word somewhere in the phrase). I would argue that for the sake of the examples we should directly use unwrap. Ditto for every instance of `expect`. ########## datasketches/src/countmin/sketch.rs: ########## @@ -54,6 +54,14 @@ impl CountMinSketch { /// /// Panics if `num_hashes` is 0, `num_buckets` is less than 3, or the /// total table size exceeds the supported limit. + /// + /// # Examples + /// + /// ``` Review Comment: Yes, just to be specific even though it seems like it's the default. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
