DOC fix typos in the user guide linear model documentation #19554
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ogrisel
reviewed
Feb 25, 2021
There was a problem hiding this comment.
Those are not all "typos". Many of the suggested changes are actually changing the meaning of the edited sentences. Please do not suggest changes if you are not confident the original meaning is preserved.
It takes time to review pull requests and if 80% of the suggestions are problematic this is really not a productive way to contribute to open source.
doc/modules/linear_model.rst
Outdated
| columns of the design matrix :math:`X` have an approximate linear | ||
| dependence, the design matrix becomes close to singular | ||
| columns of the design matrix :math:`X` have an approximately linear | ||
| dependence, the design matrix becomes close to a singular |
There was a problem hiding this comment.
Suggested change
| dependence, the design matrix becomes close to a singular | |
| dependence, the design matrix becomes close to singular |
singular is an adjective here.
doc/modules/linear_model.rst
Outdated
| @@ -546,7 +546,7 @@ the residual. | |||
| Instead of giving a vector result, the LARS solution consists of a | |||
| curve denoting the solution for each value of the :math:`\ell_1` norm of the | |||
| parameter vector. The full coefficients path is stored in the array | |||
| ``coef_path_``, which has size (n_features, max_features+1). The first | |||
| ``coef_path_``, which has a size (n_features, max_features+1). The first | |||
There was a problem hiding this comment.
The previous version seems more correct to me.
Suggested change
| ``coef_path_``, which has a size (n_features, max_features+1). The first | |
| ``coef_path_``, which has size (n_features, max_features+1). The first |
| @@ -1,6 +1,6 @@ | |||
| """ | |||
| ================================================================== | |||
| Common pitfalls in interpretation of coefficients of linear models | |||
| Common pitfalls in the interpretation of coefficients of linear models | |||
| ================================================================== | |||
There was a problem hiding this comment.
Please adjust the size of the header over/underline accordingly.
doc/modules/linear_model.rst
Outdated
Comment on lines
503
to
504
| The LARS model can be used using the estimator :class:`Lars`, or its | ||
| low-level implementation :func:`lars_path` or :func:`lars_path_gram`. |
There was a problem hiding this comment.
Suggested change
| The LARS model can be used using the estimator :class:`Lars`, or its | |
| low-level implementation :func:`lars_path` or :func:`lars_path_gram`. | |
| The LARS model can be used via the :class:`Lars` estimator, or its | |
| low-level implementation :func:`lars_path` or :func:`lars_path_gram`. |
doc/modules/linear_model.rst
Outdated
| between the features. | ||
|
|
||
| The advantages of LARS are: | ||
|
|
||
| - It is numerically efficient in contexts where the number of features | ||
| is significantly greater than the number of samples. | ||
|
|
||
| - It is computationally just as fast as forward selection and has | ||
| - It is computationally just as fast as forwarding selection and has |
There was a problem hiding this comment.
This is wrong. "forward selection" is the correct name of the method.
Suggested change
| - It is computationally just as fast as forwarding selection and has | |
| - It is computationally just as fast as forward selection and has |
doc/modules/linear_model.rst
Outdated
| @@ -306,10 +306,10 @@ Alternatively, the estimator :class:`LassoLarsIC` proposes to use the | |||
| Akaike information criterion (AIC) and the Bayes Information criterion (BIC). | |||
| It is a computationally cheaper alternative to find the optimal value of alpha | |||
| as the regularization path is computed only once instead of k+1 times | |||
| when using k-fold cross-validation. However, such criteria needs a | |||
| when using k-fold cross-validation. However, such criteria need a | |||
There was a problem hiding this comment.
The original was correct.
Suggested change
| when using k-fold cross-validation. However, such criteria need a | |
| when using k-fold cross-validation. However, such criteria needs a |
doc/modules/linear_model.rst
Outdated
| target. When there are multiple features having equal correlation, instead | ||
| of continuing along the same feature, it proceeds in a direction equiangular | ||
| target. When multiple features having equal correlation, instead | ||
| of continuing along with the same feature, it proceeds in a direction equiangular |
There was a problem hiding this comment.
Here the previous text was better. Please revert.
doc/modules/linear_model.rst
Outdated
| @@ -530,7 +530,7 @@ function of the norm of its coefficients. |
|
||
| * :ref:`sphx_glr_auto_examples_linear_model_plot_lasso_lars.py` | |||
|
|
|||
| The Lars algorithm provides the full path of the coefficients along | |||
| The Lars algorithm provides the full path of the coefficients along with | |||
| the regularization parameter almost for free, thus a common operation | |||
There was a problem hiding this comment.
The suggestion changes the meaning of then sentence. Please revert.
doc/modules/linear_model.rst
Outdated
| @@ -565,13 +565,13 @@ algorithm for approximating the fit of a linear model with constraints imposed | |||
| on the number of non-zero coefficients (ie. the :math:`\ell_0` pseudo-norm). | |||
|
|
|||
| Being a forward feature selection method like :ref:`least_angle_regression`, | |||
| orthogonal matching pursuit can approximate the optimum solution vector with a | |||
| the orthogonal matching pursuit can approximate the optimum solution vector with a | |||
There was a problem hiding this comment.
orthogonal matching pursuit is the name of the method. The previous text was correct.
doc/modules/linear_model.rst
Outdated
| fixed number of non-zero elements: | ||
|
|
||
| .. math:: | ||
| \underset{w}{\operatorname{arg\,min\,}} ||y - Xw||_2^2 \text{ subject to } ||w||_0 \leq n_{\text{nonzero\_coefs}} | ||
|
|
||
| Alternatively, orthogonal matching pursuit can target a specific error instead | ||
| Alternatively,the orthogonal matching pursuit can target a specific error instead |
There was a problem hiding this comment.
orthogonal matching pursuit is the name of the method. The previous text was correct.
|
I review and applied the suggestion of @ogrisel to merge this PR. |
|
All green. Merging. |
TomDLT
pushed a commit
to TomDLT/scikit-learn
that referenced
this pull request
Jul 29, 2021
…arn#19554) Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
samronsin
pushed a commit
to samronsin/scikit-learn
that referenced
this pull request
Nov 30, 2021
…arn#19554) Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues/PRs
What does this implement/fix? Explain your changes.
Any other comments?