Skip to content
This repository was archived by the owner on Jul 7, 2020. It is now read-only.

Corrected accuracy equation, added withAccuracy builder method#157

Merged
tedpearson merged 1 commit intoaddthis:masterfrom
kujon:with_accuracy
Aug 27, 2019
Merged

Corrected accuracy equation, added withAccuracy builder method#157
tedpearson merged 1 commit intoaddthis:masterfrom
kujon:with_accuracy

Conversation

@kujon
Copy link
Copy Markdown
Contributor

@kujon kujon commented Jun 7, 2019

Hi there 👋

First of all, what an amazing collection of utilities, super useful.

I noticed a few things I thought I improve:

  1. What is referred to in the accuracy equation as accuracy is really the error (1 - accuracy).
  2. The existing constructors require developers to have an understanding of log2m and relative standard deviation to be able to choose the value that is right for them.

This pull request aims to improve that by:

  1. Correcting the comment
  2. Introducing withAccuracy method to the builder

Copy link
Copy Markdown
Contributor

@abramsm abramsm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! These are great additions to the class.

@kujon
Copy link
Copy Markdown
Contributor Author

kujon commented Jun 28, 2019

You're welcome @abramsm! Any chance of getting it merged reasonably soon?

@tedpearson
Copy link
Copy Markdown
Contributor

Sorry for the delay. We'll try to release this in version 2.9.7 shortly.

@tedpearson tedpearson merged commit c901ea4 into addthis:master Aug 27, 2019
@tedpearson
Copy link
Copy Markdown
Contributor

@kujon 2.9.8 is now available on maven central. (2.9.7 had an incorrect sources jar so we made another release.)

@kujon
Copy link
Copy Markdown
Contributor Author

kujon commented Aug 28, 2019

Amazing @tedpearson , thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants