tisonkun commented on code in PR #155:
URL: https://github.com/apache/datasketches-rust/pull/155#discussion_r3651440090
##########
datasketches/src/theta/a_not_b.rs:
##########
Review Comment:
@leerho Thanks for your information!
Yes the Rust impl also defines the API as:
```rust
let update_policy = DefaultUpdatePolicy::<u64>::default();
let mut a = TupleSketchBuilder::new(update_policy).build();
a.update("apple", 1);
a.update("banana", 1);
let mut b = TupleSketchBuilder::new(update_policy).build();
b.update("banana", 1);
let a_not_b = TupleAnotB::default();
let result = a_not_b.compute(&a, &b, true).unwrap();
```
So I generally agree that `a_not_b` provides more information and avoid the
confusion of op direction.
Even if we can add methods or operator overrides to
`ThetaSketch`/`TupleSketch`, it is not symmetry to the union and intersection
interface.
One minor comment: any typical reason to name it `TupleAnotB` rather than
`TupleANotB`? Weird `CamelCase` at first glance.
--
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]
