WIP fix Naive Bayes coefficient stuff #2250

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

Owner

This fixes #2237 and #2240.

Not yet MRG and 0.14-rc because I'm not sure if there's a good way to preserve backward compat and some more refactoring needs to be done.

@ogrisel ogrisel and 1 other commented on an outdated diff Jul 26, 2013
sklearn/naive_bayes.py
@@ -48,7 +49,7 @@ def _joint_log_likelihood(self, X):
predict_proba and predict_log_proba.
"""
- def predict(self, X):
+ def _predict(self, X):
ogrisel
ogrisel Jul 26, 2013 Owner

This method is no longer used at all?

larsmans
larsmans Jul 26, 2013 Owner

I thought I'd removed that. It moved to GaussianNB, while the other two use LinearClassifierMixin.predict which is faster in the binary case.

@ogrisel ogrisel commented on the diff Jul 26, 2013
sklearn/naive_bayes.py
@@ -340,21 +365,8 @@ def fit(self, X, y, sample_weight=None, class_prior=None):
self._update_class_log_prior(class_prior=class_prior)
ogrisel
ogrisel Jul 26, 2013 Owner

I think those two functions should not be collapsed into one private method, eg. _update_smoothed_params or similar.

larsmans
larsmans Jul 26, 2013 Owner

Indeed. The calling order matters now, so they should be a single _update_parameters.

Owner

What is the status of this? It looks kinda old...

@larsmans larsmans closed this Mar 8, 2016
@larsmans larsmans deleted the larsmans:fix-nb-coef branch Mar 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment