Skip to content

[MRG + 1] return_train_score deprecation#12241

Merged
ogrisel merged 5 commits intoscikit-learn:masterfrom
amueller:return_train_score_deprecation
Oct 3, 2018
Merged

[MRG + 1] return_train_score deprecation#12241
ogrisel merged 5 commits intoscikit-learn:masterfrom
amueller:return_train_score_deprecation

Conversation

@amueller
Copy link
Copy Markdown
Member

@amueller amueller commented Oct 1, 2018

Part of #11992.
Should we remove the DeprecationDict class as well?

@ogrisel
Copy link
Copy Markdown
Member

ogrisel commented Oct 1, 2018

Should we remove the DeprecationDict class as well?

+1 if it's not used anywhere else.

@sklearn-lgtm
Copy link
Copy Markdown

sklearn-lgtm commented Oct 1, 2018

This pull request introduces 3 alerts when merging ca6fd96 into 59b15c5 - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Unused local variable

Comment posted by LGTM.com

@sklearn-lgtm
Copy link
Copy Markdown

sklearn-lgtm commented Oct 2, 2018

This pull request fixes 1 alert when merging 1d9b6bb into dfd009d - view on LGTM.com

fixed alerts:

  • 1 for __eq__ not overridden when adding attributes

Comment posted by LGTM.com

@amueller amueller mentioned this pull request Oct 2, 2018
Merged
@sklearn-lgtm
Copy link
Copy Markdown

sklearn-lgtm commented Oct 2, 2018

This pull request fixes 1 alert when merging 2042ded into dfd009d - view on LGTM.com

fixed alerts:

  • 1 for __eq__ not overridden when adding attributes

Comment posted by LGTM.com

@amueller amueller changed the title return_train_score deprecation [MRG] return_train_score deprecation Oct 2, 2018
rth
rth approved these changes Oct 2, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@rth rth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! A minor comment (line numbers are for this branch at its current version);

  • sklearn/model_selection/_validation.py:383 possibly remove return_train_score=False since it's the default.

@amueller
Copy link
Copy Markdown
Member Author

amueller commented Oct 2, 2018

done, thanks @rth :)

@sklearn-lgtm
Copy link
Copy Markdown

sklearn-lgtm commented Oct 2, 2018

This pull request fixes 1 alert when merging 9954544 into dfd009d - view on LGTM.com

fixed alerts:

  • 1 for __eq__ not overridden when adding attributes

Comment posted by LGTM.com

@amueller amueller changed the title [MRG] return_train_score deprecation [MRG + 1] return_train_score deprecation Oct 2, 2018
@ogrisel ogrisel merged commit fb7be87 into scikit-learn:master Oct 3, 2018
@ogrisel
Copy link
Copy Markdown
Member

ogrisel commented Oct 3, 2018

Merged, thanks for fixing this annoying warning :)

@amueller
Copy link
Copy Markdown
Member Author

amueller commented Oct 3, 2018

well the warning came only if you accessed it, right? Though I guess if you convert the dict to a dataframe it's there every time :-/

Wanna review the other ones lol?

@jrbourbeau jrbourbeau mentioned this pull request Oct 4, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rth rth rth approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@amueller @ogrisel @sklearn-lgtm @rth