Skip to content

[MRG] Speed up plot_discretization_classification.py #21661

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Nov 16, 2021
Merged

[MRG] Speed up plot_discretization_classification.py #21661

merged 7 commits into from
Nov 16, 2021

Conversation

yuanx749
Copy link
Contributor

Reference Issues/PRs

#21598

What does this implement/fix? Explain your changes.

I reduce the parameter search space.

Another issue: the original code standardizes the entire dataset, which may cause data leakage. I moved the StandardScaler into the pipelines.

before: 28s
before
after: 6s
after

Any other comments?

@adrinjalali adrinjalali mentioned this pull request Nov 15, 2021
Closed
41 tasks
adrinjalali
adrinjalali reviewed Nov 15, 2021
View reviewed changes
Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Overall nice improvement, I wouldn't have changed c_, but I also don't mind the change.

And nice catch on the StandardScaler issue.

adrinjalali
adrinjalali reviewed Nov 15, 2021
View reviewed changes
@adrinjalali
Copy link
Member

you need to apply black on your code to be safe that it always passes our CI.

Also, please merge with the latest main to pass the CI.

@yuanx749
Copy link
Contributor Author

I forgot to merge the latest main. There is still one failing......

@adrinjalali adrinjalali requested a review from TomDLT November 16, 2021 11:05
TomDLT
TomDLT approved these changes Nov 16, 2021
View reviewed changes
Copy link
Member

@TomDLT TomDLT left a comment

Choose a reason for hiding this comment

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

Great, thanks !

@TomDLT TomDLT merged commit 0526c7d into scikit-learn:main Nov 16, 2021
@yuanx749 yuanx749 deleted the plot_discretization_classification branch November 16, 2021 17:50
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Nov 22, 2021
@yuanx749 @glemaitre
ENH Speed up plot_discretization_classification.py (scikit-learn#21661)
86c57e7
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Nov 29, 2021
@yuanx749 @glemaitre
ENH Speed up plot_discretization_classification.py (scikit-learn#21661)
68bee42
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
@yuanx749 @samronsin
ENH Speed up plot_discretization_classification.py (scikit-learn#21661)
2b31e55
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Dec 24, 2021
@yuanx749 @glemaitre
ENH Speed up plot_discretization_classification.py (scikit-learn#21661)
3af008f
glemaitre pushed a commit that referenced this pull request Dec 25, 2021
@yuanx749 @glemaitre
ENH Speed up plot_discretization_classification.py (#21661)
00d3b73
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali approved these changes

@TomDLT TomDLT TomDLT approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yuanx749 @adrinjalali @TomDLT