Implementing Rosy's suggestions to code

Author: rvasav26

docs/src/references/pcovr_decomposition.rst renamed to docs/src/references/decomposition.rst

@@ -1,5 +1,5 @@
-Principal Covariates Regression (PCovR)
-================================================================
+Principal Covariates Regression (PCovR) and Classification (PCovC)
+==================================================================
 
 .. _PCovR-api:
 
@@ -20,6 +20,26 @@ PCovR
     .. automethod:: inverse_transform
     .. automethod:: score
 
+.. _PCovC-api:
+
+PCovC
+-----
+
+.. autoclass:: skmatter.decomposition.PCovC
+    :show-inheritance:
+    :special-members:
+
+    .. automethod:: fit
+
+        .. automethod:: _fit_feature_space
+        .. automethod:: _fit_sample_space
+
+    .. automethod:: transform
+    .. automethod:: predict
+    .. automethod:: inverse_transform
+    .. automethod:: decision_function
+    .. automethod:: score
+
 .. _KPCovR-api:
 
 Kernel PCovR

docs/src/references/index.rst

@@ -11,8 +11,7 @@ API Reference
   selection
   linear_models
   clustering
-  pcovc_decomposition
-  pcovr_decomposition
+  decomposition
   metrics
   neighbors
   datasets

docs/src/references/pcovc_decomposition.rst

@@ -0,0 +1,2 @@
+PCovC
+=====

examples/pcovc/README.rst

@@ -38,7 +38,7 @@ def __init__(
         self.random_state = random_state
         self.whiten = whiten
 
-    def _fit_utils(self, X):
+    def fit(self, X):
         """Contains the common functionality for the PCovR and PCovC fit methods,
         but leaves the rest of the functionality to the subclass.
         """

src/skmatter/decomposition/_pcov.py

@@ -149,10 +149,10 @@ class PCovC(LinearClassifierMixin, _BasePCov):
         The linear classifier passed for fitting.
 
     z_classifier_ : estimator object
-        The linear classifier fit between X and Y.
+        The linear classifier fit between :math:`\mathbf{X}` and :math:`\mathbf{Y}`.
 
     classifier_ : estimator object
-        The linear classifier fit between T and Y.
+        The linear classifier fit between :math:`\mathbf{T}` and  :math:`\mathbf{Y}`.
 
     pxt_ : ndarray of size :math:`({n_{features}, n_{components}})`
         the projector, or weights, from the input space :math:`\mathbf{X}`
@@ -239,13 +239,28 @@ def fit(self, X, Y, W=None):
         W : numpy.ndarray, shape (n_features, n_properties)
             Classification weights, optional when classifier=`precomputed`. If
             not passed, it is assumed that the weights will be taken from a
-            linear classifier fit between X and Y
+            linear classifier fit between :math:`\mathbf{X}` and :math:`\mathbf{Y}`
+
+        Notes
+        -----
+        Note the relationship between :math:`\mathbf{X}`, :math:`\mathbf{Y}`,
+        :math:`\mathbf{Z}`, and :math:`\mathbf{W}`. The classification weights
+        :math:`\mathbf{W}`, obtained through a linear classifier fit between
+        :math:`\mathbf{X}` and :math:`\mathbf{Y}`, are used to compute:
+
+        .. math::
+            \mathbf{Z} = \mathbf{X} \mathbf{W}
+
+        Next, :math:`\mathbf{Z}` is used in either `_fit_feature_space` or
+        `_fit_sample_space` as our approximation of :math:`\mathbf{Y}`.
+        Finally, we refit a classifier on :math:`\mathbf{T}` and :math:`\mathbf{Y}`
+        to obtain :math:`\mathbf{P}_{XZ}` and :math:`\mathbf{P}_{TZ}`
         """
         X, Y = validate_data(self, X, Y, y_numeric=False)
         check_classification_targets(Y)
         self.classes_ = np.unique(Y)
 
-        super()._fit_utils(X)
+        super().fit(X)
 
         compatible_classifiers = (
             LogisticRegression,

src/skmatter/decomposition/_pcovc.py

@@ -229,7 +229,7 @@ def fit(self, X, Y, W=None):
             passed, it is assumed that `W = np.linalg.lstsq(X, Y, self.tol)[0]`
         """
         X, Y = validate_data(self, X, Y, y_numeric=True, multi_output=True)
-        super()._fit_utils(X)
+        super().fit(X)
 
         compatible_regressors = (LinearRegression, Ridge, RidgeCV)
 

src/skmatter/decomposition/_pcovr.py

@@ -461,10 +461,10 @@ def test_prefit_classifier(self):
     def test_precomputed_classification(self):
         classifier = LogisticRegression()
         classifier.fit(self.X, self.Y)
-        Yhat = classifier.predict(self.X)
+
         W = classifier.coef_.T.reshape(self.X.shape[1], -1)
         pcovc1 = self.model(mixing=0.5, classifier="precomputed", n_components=1)
-        pcovc1.fit(self.X, Yhat, W)
+        pcovc1.fit(self.X, self.Y, W)
         t1 = pcovc1.transform(self.X)
 
         pcovc2 = self.model(mixing=0.5, classifier=classifier, n_components=1)
@@ -473,6 +473,14 @@ def test_precomputed_classification(self):
 
         self.assertTrue(np.linalg.norm(t1 - t2) < self.error_tol)
 
+        # Now check for match when W is not passed:
+        pcovc3 = self.model(mixing=0.5, classifier="precomputed", n_components=1)
+        pcovc3.fit(self.X, self.Y)
+        t3 = pcovc3.transform(self.X)
+
+        self.assertTrue(np.linalg.norm(t3 - t2) < self.error_tol)
+        self.assertTrue(np.linalg.norm(t3 - t1) < self.error_tol)
+
     def test_classifier_modifications(self):
         classifier = LogisticRegression()
         pcovc = self.model(mixing=0.5, classifier=classifier)