-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
Remove duplicate GaussianNB.fit() code #3344
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
Conversation
This code is substantially duplicated by partial_fit(), so just call partial_fit().
@@ -235,7 +215,7 @@ def _update_mean_variance(n_past, mu, var, X): | |||
|
|||
return total_sum / n_total, total_ssd / n_total | |||
|
|||
def partial_fit(self, X, y, classes=None): | |||
def partial_fit(self, X, y, classes=None, _refit=False): |
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.
adding such a param is not ok. The API shall stay clean. You might rely a private _partial_fit method.
OK, preserved API using your suggestion in d709833. |
Must be provided at the first call to partial_fit, can be omitted | ||
in subsequent calls. | ||
|
||
_refit: boolean |
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.
boolean -> bool
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.
fixed in 2d12952
8000
LGTM if travis is happy |
Remove duplicate GaussianNB.fit() code
thanks |
Follow-on to #3324.
GaussianNB.fit
is substantially duplicated bypartial_fit
, and the latter is more flexible, so just have the former call the latter. There should be no significant degradation in performance or numerical stability, since the mean/variance update will just callnumpy.mean
andnumpy.var
whenn_past == 0
rather than trying anything fancy.