Repository: scikit-learn/scikit-learn
Pull request: FIX #13117: implement fit methods in subclasses of LinearModelCV
<!--
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/main/CONTRIBUTING.md
-->

#### 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
-->

Fixes #13117
See also #13131

#### What does this implement/fix? Explain your changes.

As suggested in #13131 this PR implements `fit()` methods that just call `super()` in all subclasses of `LinearModelCV` just to have a better documentation (see original issue #13117 for context of the potential problems). This also opens up the way of slightly altering the docstrings so they are better tailored for the specific classes.

#### 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:
https://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
Possible error in documentation of ElasticNetCV()
The documentation for class `ElasticNetCV()` reads:

    y : array-like, shape (n_samples,) or (n_samples, n_targets)

However, when I try to pass a 2D array I get the error:

```
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-33-597417f0a72a> in <module>()
      2 
      3 enet = ElasticNetCV()
----> 4 enet.fit(X_train, y_train)

/usr/local/lib/python3.6/dist-packages/sklearn/linear_model/coordinate_descent.py in fit(self, X, y)
   1091             if y.ndim > 1 and y.shape[1] > 1:
   1092                 raise ValueError("For multi-task outputs, use "
-> 1093                                  "MultiTask%sCV" % (model_str))
   1094             y = column_or_1d(y, warn=True)
   1095         else:

ValueError: For multi-task outputs, use MultiTaskElasticNetCV
```

Hints from issue discussion:
would you like to submit a PR to fix it @albertotb ?
Sure, I'll get to it

On Sat, Feb 9, 2019 at 2:51 PM Adrin Jalali <notifications@github.com>
wrote:

> would you like to submit a PR to fix it @albertotb
> <https://github.com/albertotb> ?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/scikit-learn/scikit-learn/issues/13117#issuecomment-462046154>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AGNoAmCxvvy-SsE7OnYRLj_CvUdolaWLks5vLtJRgaJpZM4avYAl>
> .
>

@adrinjalali 

> y : array-like, shape (n_samples,) or (n_samples, n_targets)
>     Target values

Will this be solved if  y is modified to

> y : array_like ,shape (n_samples)




@maverick100 not really, please refer to https://github.com/scikit-learn/scikit-learn/pull/13131 for a potential solution.
Is the current state of this PR overruled by #13131 or does it need a fresh start?
Seems like it's blocked on the reviewer side @vaibhavmehrotraml 
Hi is this issue resolved? would like to take a stab at it if not
The issue still exists. 
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-15-6595eecb0bff> in <module>
      1 regr = ElasticNetCV(cv=5, random_state=0)
----> 2 regr.fit(X, y)
      3 ElasticNetCV(cv=5, random_state=0)

E:\programs\anaconda\envs\virtual_workspace\lib\site-packages\sklearn\linear_model\_coordinate_descent.py in fit(self, X, y)
   1244         if not self._is_multitask():
   1245             if y.ndim > 1 and y.shape[1] > 1:
-> 1246                 raise ValueError("For multi-task outputs, use "
   1247                                  "MultiTask%s" % self.__class__.__name__)
   1248             y = column_or_1d(y, warn=True)

ValueError: For multi-task outputs, use MultiTaskElasticNetCV

-----------------------------------------------------------------
Documentation says:
    y array-like of shape (n_samples,) or (n_samples, n_targets)
    Target values.
Potential solution:
    y array-like of shape (n_samples,)
    or
    y array-like of shape (n_samples,) or (n_samples, n_targets [use MultiTaskElasticNetCV] ) 

I'd like to contribute with this one! Where can I find the documentation files?
@lucastosetto this may be fixed by #13131, but the PR is a bit stale. You can start by looking at the changes there

Commit message: remove duplicate fits

Patch to review:
diff --git a/sklearn/linear_model/_coordinate_descent.py b/sklearn/linear_model/_coordinate_descent.py
index 4ede6c04b462b..544cdb1412cc6 100644
--- a/sklearn/linear_model/_coordinate_descent.py
+++ b/sklearn/linear_model/_coordinate_descent.py
@@ -2083,6 +2083,46 @@ def _is_multitask(self):
     def _more_tags(self):
         return {"multioutput": False}
 
+    def fit(self, X, y, sample_weight=None, **params):
+        """Fit Lasso model with coordinate descent.
+
+        Fit is on grid of alphas and best alpha estimated by cross-validation.
+
+        Parameters
+        ----------
+        X : {array-like, sparse matrix} of shape (n_samples, n_features)
+            Training data. Pass directly as Fortran-contiguous data
+            to avoid unnecessary memory duplication. If y is mono-output,
+            X can be sparse. Note that large sparse matrices and arrays
+            requiring `int64` indices are not accepted.
+
+        y : array-like of shape (n_samples,)
+            Target values.
+
+        sample_weight : float or array-like of shape (n_samples,), \
+                default=None
+            Sample weights used for fitting and evaluation of the weighted
+            mean squared error of each cv-fold. Note that the cross validated
+            MSE that is finally used to find the best model is the unweighted
+            mean over the (weighted) MSEs of each test fold.
+
+        **params : dict, default=None
+            Parameters to be passed to the CV splitter.
+
+            .. versionadded:: 1.4
+                Only available if `enable_metadata_routing=True`,
+                which can be set by using
+                ``sklearn.set_config(enable_metadata_routing=True)``.
+                See :ref:`Metadata Routing User Guide <metadata_routing>` for
+                more details.
+
+        Returns
+        -------
+        self : object
+            Returns an instance of fitted model.
+        """
+        super().fit(X, y, sample_weight=sample_weight, **params)
+
 
 class ElasticNetCV(RegressorMixin, LinearModelCV):
     """Elastic Net model with iterative fitting along a regularization path.
@@ -2322,6 +2362,46 @@ def _is_multitask(self):
     def _more_tags(self):
         return {"multioutput": False}
 
+    def fit(self, X, y, sample_weight=None, **params):
+        """Fit ElasticNet model with coordinate descent.
+
+        Fit is on grid of alphas and best alpha estimated by cross-validation.
+
+        Parameters
+        ----------
+        X : {array-like, sparse matrix} of shape (n_samples, n_features)
+            Training data. Pass directly as Fortran-contiguous data
+            to avoid unnecessary memory duplication. If y is mono-output,
+            X can be sparse. Note that large sparse matrices and arrays
+            requiring `int64` indices are not accepted.
+
+        y : array-like of shape (n_samples,)
+            Target values.
+
+        sample_weight : float or array-like of shape (n_samples,), \
+                default=None
+            Sample weights used for fitting and evaluation of the weighted
+            mean squared error of each cv-fold. Note that the cross validated
+            MSE that is finally used to find the best model is the unweighted
+            mean over the (weighted) MSEs of each test fold.
+
+        **params : dict, default=None
+            Parameters to be passed to the CV splitter.
+
+            .. versionadded:: 1.4
+                Only available if `enable_metadata_routing=True`,
+                which can be set by using
+                ``sklearn.set_config(enable_metadata_routing=True)``.
+                See :ref:`Metadata Routing User Guide <metadata_routing>` for
+                more details.
+
+        Returns
+        -------
+        self : object
+            Returns an instance of fitted model.
+        """
+        super().fit(X, y, sample_weight=sample_weight, **params)
+
 
 ###############################################################################
 # Multi Task ElasticNet and Lasso models (with joint feature selection)
@@ -2943,7 +3023,7 @@ def _more_tags(self):
         return {"multioutput_only": True}
 
     # This is necessary as LinearModelCV now supports sample_weight while
-    # MultiTaskElasticNet does not (yet).
+    # MultiTaskElasticNetCV does not (yet).
     def fit(self, X, y, **params):
         """Fit MultiTaskElasticNet model with coordinate descent.
 
@@ -3181,7 +3261,7 @@ def _more_tags(self):
         return {"multioutput_only": True}
 
     # This is necessary as LinearModelCV now supports sample_weight while
-    # MultiTaskElasticNet does not (yet).
+    # MultiTaskLassoCV does not (yet).
     def fit(self, X, y, **params):
         """Fit MultiTaskLasso model with coordinate descent.
 


Code review comments:
diff_hunk: @@ -2322,6 +2362,46 @@ def _is_multitask(self):
     def _more_tags(self):
         return {"multioutput": False}
 
+    def fit(self, X, y, sample_weight=None, **params):
+        """Fit ElasticNet model with coordinate descent.
+
+        Fit is on grid of alphas and best alpha estimated by cross-validation.
+
+        Parameters
+        ----------
+        X : {array-like, sparse matrix} of shape (n_samples, n_features)
+            Training data. Pass directly as Fortran-contiguous data
+            to avoid unnecessary memory duplication. If y is mono-output,
+            X can be sparse. Note that large sparse matrices and arrays
+            requiring `int64` indices are not accepted.
+
+        y : array-like of shape (n_samples,)
+            Target values.
+
+        sample_weight : float or array-like of shape (n_samples,), \
+                default=None
+            Sample weights used for fitting and evaluation of the weighted
+            mean squared error of each cv-fold. Note that the cross validated
+            MSE that is finally used to find the best model is the unweighted
+            mean over the (weighted) MSEs of each test fold.
+
+        **params : dict, default=None
+            Parameters to be passed to the CV splitter.
+
+            .. versionadded:: 1.4
+                Only available if `enable_metadata_routing=True`,
+                which can be set by using
+                ``sklearn.set_config(enable_metadata_routing=True)``.
+                See :ref:`Metadata Routing User Guide <metadata_routing>` for
+                more details.
+
+        Returns
+        -------
+        self : object
+            Returns an instance of fitted model.
+        """
+        super().fit(X, y, sample_weight=sample_weight, **params)
path: sklearn/linear_model/_coordinate_descent.py
text: @user1:
```suggestion
        return super().fit(X, y, sample_weight=sample_weight, **params)
```

diff_hunk: @@ -2083,6 +2083,46 @@ def _is_multitask(self):
     def _more_tags(self):
         return {"multioutput": False}
 
+    def fit(self, X, y, sample_weight=None, **params):
+        """Fit Lasso model with coordinate descent.
+
+        Fit is on grid of alphas and best alpha estimated by cross-validation.
+
+        Parameters
+        ----------
+        X : {array-like, sparse matrix} of shape (n_samples, n_features)
+            Training data. Pass directly as Fortran-contiguous data
+            to avoid unnecessary memory duplication. If y is mono-output,
+            X can be sparse. Note that large sparse matrices and arrays
+            requiring `int64` indices are not accepted.
+
+        y : array-like of shape (n_samples,)
+            Target values.
+
+        sample_weight : float or array-like of shape (n_samples,), \
+                default=None
+            Sample weights used for fitting and evaluation of the weighted
+            mean squared error of each cv-fold. Note that the cross validated
+            MSE that is finally used to find the best model is the unweighted
+            mean over the (weighted) MSEs of each test fold.
+
+        **params : dict, default=None
+            Parameters to be passed to the CV splitter.
+
+            .. versionadded:: 1.4
+                Only available if `enable_metadata_routing=True`,
+                which can be set by using
+                ``sklearn.set_config(enable_metadata_routing=True)``.
+                See :ref:`Metadata Routing User Guide <metadata_routing>` for
+                more details.
+
+        Returns
+        -------
+        self : object
+            Returns an instance of fitted model.
+        """
+        super().fit(X, y, sample_weight=sample_weight, **params)
path: sklearn/linear_model/_coordinate_descent.py
text: @user1:
```suggestion
        return super().fit(X, y, sample_weight=sample_weight, **params)
```