Enhance docstring for LinearRegression.fit #28741
Conversation
sklearn/linear_model/_base.py
Outdated
| @@ -560,8 +560,15 @@ def fit(self, X, y, sample_weight=None): | |||
| y : array-like of shape (n_samples,) or (n_samples, n_targets) | |||
| Target values. Will be cast to X's dtype if necessary. | |||
|
|
|||
| sample_weight : array-like of shape (n_samples,), default=None | |||
| Individual weights for each sample. | |||
| sample_weight : array-like of shape (n_samples,), int, float or None (default) | |||
There was a problem hiding this comment.
This seems more consistent with how other docstrings are formatted
| sample_weight : array-like of shape (n_samples,), int, float or None (default) | |
| sample_weight : array-like of shape (n_samples,), int or float, default=None |
I could only find None (default) in the actual text explaining a parameter, not in the "headline"
sklearn/linear_model/_base.py
Outdated
| - `int` or `float`: all samples have a weight equal to the value \ | ||
| provided. Since there is no difference in the relative weight between samples, \ | ||
| results in the same fitted model as when `sample_weight=None`. |
There was a problem hiding this comment.
I'd go for something concise like
| - `int` or `float`: all samples have a weight equal to the value \ | |
| provided. Since there is no difference in the relative weight between samples, \ | |
| results in the same fitted model as when `sample_weight=None`. | |
| - `int` or `float`: all samples have a weight equal to the value provided. |
I wonder if we should explain to the user what exact effect (no effect) this has, we don't go into that for array-like either. So maybe we can skip it.
|
Thanks for the PR fixing this! I left two small comments about formatting and level of detail |
|
While this PR add technically correct statements, I'm not so sure to include it. Also, if we start here, do we go through all estimators to adapt the description? |
|
I would not add the detailed paragraph either. But I'd still fix the valid types. And I think we should in all places where it applies (floats are not always supported), in other PRs though. |
sklearn/linear_model/_base.py
Outdated
| @@ -560,8 +560,15 @@ def fit(self, X, y, sample_weight=None): | |||
| y : array-like of shape (n_samples,) or (n_samples, n_targets) | |||
| Target values. Will be cast to X's dtype if necessary. | |||
|
|
|||
| sample_weight : array-like of shape (n_samples,), default=None | |||
| Individual weights for each sample. | |||
| sample_weight : array-like of shape (n_samples,), int, float or None (default) | |||
There was a problem hiding this comment.
no need to have float and int. float covers both. In the few places where we already mention float as valid type, it comes first. Let's keep the same formating for consistency.
| sample_weight : array-like of shape (n_samples,), int, float or None (default) | |
| sample_weight : float or array-like of shape (n_samples,), default=None |
|
The discussion on whether or not to do this is happening in #28732 |
Reference Issues/PRs
Fixes #28732
What does this implement/fix? Explain your changes.
Changes the docstring to include all types that can be used for the
sample_weightparameter, as well as explaining the user what happens in each type.