Style improvements to least_angle.py #11703
Conversation
sklearn/linear_model/least_angle.py
Outdated
| @@ -196,7 +196,7 @@ def lars_path(X, y, Xy=None, Gram=None, max_iter=500, | |||
|
|
|||
| if verbose: | |||
| if verbose > 1: | |||
| print("Step\t\tAdded\t\tDropped\t\tActive set size\t\tC") | |||
| print('Step\t\tAdded\t\tDropped\t\tActive set size\t\tC') | |||
There was a problem hiding this comment.
On what basis is this a worthwhile change?
Please don't waste reviewers' time on conforming to a personal preference.
Such changes may also introduce merge conflicts to other substantive pull requests.
There was a problem hiding this comment.
@jnothman Thanks for looking at this PR. The goal of this change certainly isn't to waste a reviewer's time!
I don't have a preference between single quotes and double quotes but I follow Google Style Guide's recommendation to "be consistent with your choice of string quote character within a file" (https://github.com/google/styleguide/blob/gh-pages/pyguide.md#310-strings). Since there are more places in this module that uses ' than " I converted those double quotes to single quotes. Of course the Google Style Guide isn't as well accepted as PEP8 which only recommends "Pick a rule and stick to it." So what's the rule here?
|
Our rule is to not be pedantic on quotes and to not fix style issues in
existing code unless clearly beneficial
|
There was a problem hiding this comment.
Honestly most of these changes (except the ones I marked as OK) are cosmetic. We don't tend to accept such PRs as they require reviews and time for little added value. Also unless there is CIs testing for style as we have with flake8 someone is likely to revert these changes. So I am -1 for the PR as it is. Hope you can understand our reasons.
| @@ -606,7 +606,8 @@ def __init__(self, fit_intercept=True, verbose=False, normalize=True, | |||
| self.copy_X = copy_X | |||
| self.fit_path = fit_path | |||
|
|
|||
| def _get_gram(self, precompute, X, y): | |||
| @staticmethod | |||
| def _get_gram(precompute, X, y): | |||
| X, y = diabetes.data, diabetes.target | ||
| G = np.dot(X.T, X) | ||
| Xy = np.dot(X.T, y) | ||
| for method in 'lar', 'lasso': |
|
@agramfort I reverted other changes except for the ones you okay-ed. |
|
thx @yukuairoy |
What does this implement/fix? Explain your changes.
This fixes/improves the style of least_angle.py and its test test_least_angle.py.