brayniac / histogram

rust histogram and percentile stats
Apache License 2.0
46 stars 4 forks source link

Return `Error` values and add convenience functions #53

Closed asthasr closed 4 years ago

asthasr commented 4 years ago

This PR has two effects: first, it returns Error values instead of &'static str in Result types. This should make it more convenient for use with common error handling idioms (particularly anyhow). Second, it adds several convenience functions that make it easier to return a set of quantiles without cumbersome error handling:

  1. quantile_vec
  2. quartiles
  3. quintiles
brayniac commented 4 years ago

Thanks for opening this PR. I'll have a chance to review it during the week. In the meantime, it looks like it may need a pass of rustfmt to pass ci.

I'll take a closer look and provide some feedback ASAP. Thanks again!

asthasr commented 4 years ago

Yeah, my local cargo fmt seems to have different output. I'll take a look when I get a moment today :)

brayniac commented 4 years ago

This looks pretty good. I'd prefer if we could use thiserror instead, otherwise I'm happy with the changes here - it's nice to have real error types and the convenience functions provided in this PR.

asthasr commented 4 years ago

This looks pretty good. I'd prefer if we could use thiserror instead, otherwise I'm happy with the changes here - it's nice to have real error types and the convenience functions provided in this PR.

:+1: glad you like these changes. I have a few more that I'll put into another PR after this one.

brayniac commented 4 years ago

Thanks for converting to thiserror. A couple small items left inline. I'll be happy to merge once those items are resolved.