Skip to content

[MRG] Speed up plot_discretization_classification.py#21661

Merged
TomDLT merged 7 commits intoscikit-learn:mainfrom
yuanx749:plot_discretization_classification
Nov 16, 2021
Merged

[MRG] Speed up plot_discretization_classification.py#21661
TomDLT merged 7 commits intoscikit-learn:mainfrom
yuanx749:plot_discretization_classification

Conversation

@yuanx749
Copy link
Copy Markdown
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
41 tasks
Copy link
Copy Markdown
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
Copy link
Copy Markdown
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
Copy Markdown
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
Copy link
Copy Markdown
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
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Nov 29, 2021
samronsin pushed a commit to samronsin/scikit-learn that referenced this pull request Nov 30, 2021
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Dec 24, 2021
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