-
-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make quartile_int a class #570
Conversation
Hi, it seems to need to rebase as there are unrelated commits in this PR. |
c21e4ec
to
cf1c10d
Compare
Fixed that, now should include commits for this PR |
Have you tried the function yourself? |
@@ -373,6 +373,7 @@ def fit_to_sample(selected_distributions, sample, x_min, x_max): | |||
|
|||
def fit_to_quartile(dist_names, q1, q2, q3, extra_pros): | |||
error = np.inf | |||
fitted_dist = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in case we are not able to fit a thing (for instance a domain error)
self._q1 = q1 | ||
self._q2 = q2 | ||
self._q3 = q3 | ||
self.dist = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to store the distribution that we fit, this is the main reason for this refactor
self.dist = fitted_dist | ||
|
||
def _setup_observers(self): | ||
def _match_distribution_(_): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to do this so we can call the function each time parameters change
Description
Closes #547
QuartileInt
classChecklist