Repository: scikit-learn/scikit-learn
Pull request: DOC Add var_ attribute and deprecate sigma_ in GaussianNB
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/master/CONTRIBUTING.md#pull-request-checklist
-->

#### Reference Issues/PRs
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->
Closes #16760 

#### What does this implement/fix? Explain your changes.
- `sigma` means std deviation which is confusing as `GaussianNB.sigma_` is currently the variance of each feature per class

- Rename sigma_ to var_ and deprecate sigma_

#### Any other comments?


<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
In `sklearn.naive_bayes`,  `GaussianNB` model return an attribute called `sigma_`. 
In the document, `sigma_` is the variance of each feature per class, but in general, it means standard variation.  So we either change its name to `variance_` or call `sigma_`  the std of each feature per class.

Hints from issue discussion:
Line reference: https://github.com/scikit-learn/scikit-learn/blob/a84ba0c0b8b4948a5571ce1a815102c43d35cdd1/sklearn/naive_bayes.py#L156

Indeed,although the documentation states clearly that it is variance it can be a bit misleading since it is usually used for standard deviation.

In most of the code, the variance is referred as `var`  (such as https://github.com/scikit-learn/scikit-learn/blob/a84ba0c0b8b4948a5571ce1a815102c43d35cdd1/sklearn/naive_bayes.py#L240)

so it would make sense to change `sigma_` for `var_`or `variance_`.
But it would imply issues with older code...

Some implementations return the stdvar instead of variance so it could be the path taken here but that would imply more issues and more code to change.
It could be representing the upper case Sigma?
Well it could but that still does not represent variance I think? Uppercase sigma is mostly for summing. I think it is a standard uppercase 's' that can be used for variance.

My thoughts on realistic possibilities:

1. Add a `_variance` or  `_var` return equals to `_sigma` and deprecate `_sigma`
2. Leave it like this and add a warning or a focus on it in the documentation for NB


For example, Sigma can be used to represent the covariance matrix in multivariate normal distributions:

<img width="135" alt="Screen Shot 2020-04-05 at 11 28 35 AM" src="https://user-images.githubusercontent.com/5402633/78502765-afcffb00-7730-11ea-96b5-44d445b23d43.png">

While for the single variable case:

<img width="121" alt="Screen Shot 2020-04-05 at 11 29 07 AM" src="https://user-images.githubusercontent.com/5402633/78502769-b3fc1880-7730-11ea-8d3a-28b0a8728064.png">

Anyways for this case, I am okay with this being `var_`. 

On a side note, it is a little strange how the attribute names do not match the variables in the user guide: https://scikit-learn.org/stable/modules/naive_bayes.html#gaussian-naive-bayes

Currently, `theta_` is used for the mean and `sigma_` is used for the variance.
take

Commit message: Add back var_ attrib and add deprecated notice

Patch to review:
diff --git a/sklearn/naive_bayes.py b/sklearn/naive_bayes.py
index 77ddb467a4b4c..fe109761377ce 100644
--- a/sklearn/naive_bayes.py
+++ b/sklearn/naive_bayes.py
@@ -156,6 +156,13 @@ class labels known to the classifier
     sigma_ : ndarray of shape (n_classes, n_features)
         variance of each feature per class
 
+        .. deprecated:: 0.24
+            ``sigma_`` was deprecated in 0.24 and will be removed in 1.1.
+            Use var_ instead.
+
+    var_ : ndarray of shape (n_classes, n_features)
+        variance of each feature per class
+
     theta_ : ndarray of shape (n_classes, n_features)
         mean of each feature per class
 
@@ -377,7 +384,7 @@ def _partial_fit(self, X, y, classes=None, _refit=False,
             n_features = X.shape[1]
             n_classes = len(self.classes_)
             self.theta_ = np.zeros((n_classes, n_features))
-            self.sigma_ = np.zeros((n_classes, n_features))
+            self.var_ = np.zeros((n_classes, n_features))
 
             self.class_count_ = np.zeros(n_classes, dtype=np.float64)
 
@@ -405,7 +412,7 @@ def _partial_fit(self, X, y, classes=None, _refit=False,
                 msg = "Number of features %d does not match previous data %d."
                 raise ValueError(msg % (X.shape[1], self.theta_.shape[1]))
             # Put epsilon back in each time
-            self.sigma_[:, :] -= self.epsilon_
+            self.var_[:, :] -= self.epsilon_
 
         classes = self.classes_
 
@@ -429,14 +436,14 @@ def _partial_fit(self, X, y, classes=None, _refit=False,
                 N_i = X_i.shape[0]
 
             new_theta, new_sigma = self._update_mean_variance(
-                self.class_count_[i], self.theta_[i, :], self.sigma_[i, :],
+                self.class_count_[i], self.theta_[i, :], self.var_[i, :],
                 X_i, sw_i)
 
             self.theta_[i, :] = new_theta
-            self.sigma_[i, :] = new_sigma
+            self.var_[i, :] = new_sigma
             self.class_count_[i] += N_i
 
-        self.sigma_[:, :] += self.epsilon_
+        self.var_[:, :] += self.epsilon_
 
         # Update if only no priors is provided
         if self.priors is None:
@@ -449,14 +456,22 @@ def _joint_log_likelihood(self, X):
         joint_log_likelihood = []
         for i in range(np.size(self.classes_)):
             jointi = np.log(self.class_prior_[i])
-            n_ij = - 0.5 * np.sum(np.log(2. * np.pi * self.sigma_[i, :]))
+            n_ij = - 0.5 * np.sum(np.log(2. * np.pi * self.var_[i, :]))
             n_ij -= 0.5 * np.sum(((X - self.theta_[i, :]) ** 2) /
-                                 (self.sigma_[i, :]), 1)
+                                 (self.var_[i, :]), 1)
             joint_log_likelihood.append(jointi + n_ij)
 
         joint_log_likelihood = np.array(joint_log_likelihood).T
         return joint_log_likelihood
 
+    @deprecated(  # type: ignore
+        "Attribute sigma_ was deprecated in 0.24 and will be removed in"
+        "1.1 (renaming of 0.26). Use var_ instead."
+    )
+    @property
+    def sigma_(self):
+        return self.var_
+
 
 _ALPHA_MIN = 1e-10
 
diff --git a/sklearn/tests/test_naive_bayes.py b/sklearn/tests/test_naive_bayes.py
index e95eccc0c747d..518bdab386824 100644
--- a/sklearn/tests/test_naive_bayes.py
+++ b/sklearn/tests/test_naive_bayes.py
@@ -58,6 +58,15 @@ def test_gnb():
     assert_raises(ValueError, GaussianNB().partial_fit, X, y, classes=[0, 1])
 
 
+def test_gnb_var():
+    clf = GaussianNB()
+    clf.fit(X, y)
+
+    # TODO remove once sigma_ attribute is removed (GH #16760)
+    with pytest.warns(FutureWarning):
+        assert_array_equal(clf.sigma_, clf.var_)
+
+
 def test_gnb_prior():
     # Test whether class priors are properly set.
     clf = GaussianNB().fit(X, y)
@@ -76,7 +85,7 @@ def test_gnb_sample_weight():
     clf_sw = GaussianNB().fit(X, y, sw)
 
     assert_array_almost_equal(clf.theta_, clf_sw.theta_)
-    assert_array_almost_equal(clf.sigma_, clf_sw.sigma_)
+    assert_array_almost_equal(clf.var_, clf_sw.var_)
 
     # Fitting twice with half sample-weights should result
     # in same result as fitting once with full weights
@@ -86,7 +95,7 @@ def test_gnb_sample_weight():
     clf2.partial_fit(X, y, sample_weight=sw / 2)
 
     assert_array_almost_equal(clf1.theta_, clf2.theta_)
-    assert_array_almost_equal(clf1.sigma_, clf2.sigma_)
+    assert_array_almost_equal(clf1.var_, clf2.var_)
 
     # Check that duplicate entries and correspondingly increased sample
     # weights yield the same result
@@ -97,7 +106,7 @@ def test_gnb_sample_weight():
     clf_sw = GaussianNB().fit(X, y, sample_weight)
 
     assert_array_almost_equal(clf_dupl.theta_, clf_sw.theta_)
-    assert_array_almost_equal(clf_dupl.sigma_, clf_sw.sigma_)
+    assert_array_almost_equal(clf_dupl.var_, clf_sw.var_)
 
 
 def test_gnb_neg_priors():
@@ -174,13 +183,13 @@ def test_gnb_partial_fit():
     clf = GaussianNB().fit(X, y)
     clf_pf = GaussianNB().partial_fit(X, y, np.unique(y))
     assert_array_almost_equal(clf.theta_, clf_pf.theta_)
-    assert_array_almost_equal(clf.sigma_, clf_pf.sigma_)
+    assert_array_almost_equal(clf.var_, clf_pf.var_)
     assert_array_almost_equal(clf.class_prior_, clf_pf.class_prior_)
 
     clf_pf2 = GaussianNB().partial_fit(X[0::2, :], y[0::2], np.unique(y))
     clf_pf2.partial_fit(X[1::2], y[1::2])
     assert_array_almost_equal(clf.theta_, clf_pf2.theta_)
-    assert_array_almost_equal(clf.sigma_, clf_pf2.sigma_)
+    assert_array_almost_equal(clf.var_, clf_pf2.var_)
     assert_array_almost_equal(clf.class_prior_, clf_pf2.class_prior_)
 
 


Code review comments:
diff_hunk: @@ -153,6 +153,13 @@ class labels known to the classifier
     epsilon_ : float
         absolute additive value to variances
 
+    sigma_ : ndarray of shape (n_classes, n_features)
+        variance of each feature per class
+
+        .. deprecated:: 0.24
+            ``sigma_`` was deprecated in 0.24 and will be removed in 1.1.
+            Use var_ instead.
path: sklearn/naive_bayes.py
text: @user1:
```suggestion
           `sigma_` was deprecated in 0.24 and will be removed in 1.1
           (renaming of 0.26). Use `var_` instead.
```