tisonkun commented on code in PR #158:
URL: https://github.com/apache/datasketches-rust/pull/158#discussion_r3637226754
##########
datasketches/src/tuple/policy.rs:
##########
@@ -15,62 +15,64 @@
// specific language governing permissions and limitations
// under the License.
-//! Policies describing how summaries are created, updated, and combined.
+//! Policies describing how summaries are created and updated.
//!
//! A Tuple sketch keeps a user-defined summary `S` next to every retained
key. The behavior of a
//! summary is supplied externally through policy objects rather than baked
into the summary type
//! itself, so the same summary type (for example a plain `u64` or a
`Vec<f64>`) can be driven by
//! different behaviors and can carry per-instance configuration (such as the
number of values in an
//! array-of-doubles summary).
+use std::marker::PhantomData;
use std::ops::AddAssign;
-/// Defines how a summary is created and how update values are folded into it.
-///
-/// This is used by the update tuple sketch. `S` is the stored summary type
and `U` is the type of
-/// the update value, which may be a borrowed type such as `&[f64]`.
-pub trait SummaryUpdatePolicy<S, U> {
+/// Defines how summaries are created.
+pub trait SummaryPolicy {
+ /// Summary type retained alongside each key.
+ type Summary;
+
/// Creates a new summary for a key seen for the first time.
///
/// The summary should be in its identity state; the first update value is
folded in separately
- /// via [`update`](Self::update).
- fn create(&self) -> S;
+ /// via [`SummaryUpdatePolicy::update`].
+ fn create(&self) -> Self::Summary;
+}
+/// Defines how update values are folded into summaries.
+///
+/// A policy may implement this trait for multiple update types. For example,
an array policy can
+/// accept slices, vectors, or other containers while retaining a single
summary type defined by
+/// [`SummaryPolicy`].
+pub trait SummaryUpdatePolicy<U>: SummaryPolicy {
/// Folds an update value into an existing summary.
- fn update(&self, summary: &mut S, value: U);
+ fn update(&self, summary: &mut Self::Summary, value: U);
}
Review Comment:
Major rework:
1. Split SummaryUpdatePolicy to SummaryPolicy and SummaryUpdatePolicy.
2. In TupleSketch and TupleSketchBuilder, use only `P` and set the original
`S` as `P::Summary`
--
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]
