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.
MRG Deprecates 'normalize' in LinearRegression (_base.py) #17743
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MRG Deprecates 'normalize' in LinearRegression (_base.py) #17743
Changes from all commits
06de537
d1c9816
233a82e
9369ed3
2e93e06
523d588
a7b7422
15368a7
293682f
6258d1c
2f4d60a
582532a
428f1fa
a93d367
0e89ab9
97c6221
25fe971
0b3e5b5
38b9b5e
7fc22ee
86bb2ef
59e4c87
2e52377
c8d9295
15f2a1e
5a3008b
45b42e3
afc9c01
63f93cc
ca413e1
e546ec1
3886e02
da1ec7d
4e4afe7
e29932d
7f32058
82584f4
6d3ee72
ad7a508
9cceafe
74c30e0
f0eb556
798efcc
abfd95d
3cbb7b0
c8d66b5
d8ee96c
0dbc444
63424b7
117059e
5ae8ece
3aec615
ff6bec1
6d5d588
5170b2c
ac18d63
2c7f8d6
8024a5a
1d8e3de
3ee028e
73c3f70
f0d850c
04dd2b2
ec950dc
509c7c8
1ae7ead
011d751
79a82e9
91fd435
1911725
8c39982
b49d4cc
f0d81fa
0208ee9
a84ac5c
01fbdd1
54e7197
eb913f6
053ceaa
71ba0bf
4a6d7bc
10e7380
4a078f4
a66a14e
9cd4156
787fe67
b0c7bc8
b01c01d
8649118
3d14885
169dae5
d61aad0
511261e
86b1c21
80eb9e5
7aee6ea
64cf6aa
c8f0456
866ced7
505bcd9
f969696
828a3e9
f5e878c
a9403c5
4683f62
1a0dfe1
f1f21f0
620d011
270a946
48c704e
2afefc5
68aa605
c97a6be
0b035a3
48f3b87
57b8819
46d8606
45d9026
4a21c22
ca7f528
8609a9e
84f7be7
81f276d
0653714
80822f7
db71adf
e8d7396
26b729a
ba547a0
4bd9b63
32447a8
5efe0de
e9527e5
6686800
21914d0
69b0080
a149dcc
0036778
7ae55a3
81d34b4
498c430
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this test is only meant to test
LinearRegression
it should be moved tosklearn/linear_model/tests/test_base.py
. If it's meant to be extended toRidge
,Lasso
... maybe it should be move to a new file, e.g.sklearn/linear_model/tests/test_linear_model.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I recall I was proposing
sklearn/linear_model/tests/test_common.py
that is the usual way that we structure common tests for a module.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To anticipate this question, I tried to see if this test would pass with the current code for Ridge and Lasso and actually it always fails whether
with_mean
isTrue
orFalse
on dense data and it also fails withwith_mean=False
on sparse data:So the deprecation of their
normalize
option should not be implemented with the same message I believe.To keep this PR focused, let's just move this test to
sklearn/linear_model/tests/test_base.py
for now.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes @glemaitre . I answered your comment, but it must have gone lost int the flow of other comments:
#17743 (comment)
I will move it to
test_base.py
.@ogrisel failing for Ridge and Lasso might indeed be a problem as it was supposed to be extended to include them in this test. Why is this the case (for their failing)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glemaitre
There is no file:
sklearn/linear_model/tests/test_common.py
(there is:
sklearn/tests/test_common.py
, hence my previous question above).Should I create it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could create one. But it's fine to keep in sklearn/linear_model/tests/test_base.py for now. You can move this test to sklearn/linear_model/tests/test_common.py in a PR that needs to reuse it for another estimator of the
sklearn.linear_model
module.