Skip to content

[MRG] Use return_X_y=True with load_digits where appropriate#14359

Merged
rth merged 2 commits intoscikit-learn:masterfrom
tq0:fea_load_digits_tests
Jul 14, 2019
Merged

[MRG] Use return_X_y=True with load_digits where appropriate#14359
rth merged 2 commits intoscikit-learn:masterfrom
tq0:fea_load_digits_tests

Conversation

@tq0
Copy link
Copy Markdown
Contributor

@tq0 tq0 commented Jul 14, 2019

Reference Issues/PRs

Partially address #14347 (Use retrun_X_y=True when applicable in examples)

What does this implement/fix? Explain your changes.

Changes the usage of load_digits in examples and tests where appropriate.

Any other comments?

Copy link
Copy Markdown
Member

@qinhanmin2014 qinhanmin2014 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 @tq0

digits = datasets.load_digits(n_class=10)
X = digits.data
y = digits.target
X, y = datasets.load_digits(n_class=10, return_X_y=True)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

n_class=10 seems redundant

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@qinhanmin2014 thanks, I removed the n_class parameter.

@tq0 tq0 changed the title Use return_X_y=True with load_digits where appropriate [MRG] Use return_X_y=True with load_digits where appropriate Jul 14, 2019
@tq0 tq0 force-pushed the fea_load_digits_tests branch from 8254599 to 2f9b6d5 Compare July 14, 2019 14:35
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.

Thanks @tq0!

@rth rth merged commit a8efe00 into scikit-learn:master Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants