-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
DOC Rework plot_grid_search_text_feature_extraction.py example #23740
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
Conversation
|
Please try to run |
|
Ok the build is fixed but the plotly output does not show-up. There is some additional configuration required, see for instance: |
ogrisel
left a comment
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.
Something is wrong with the doc-min-dependencies builder. It's been running for more than 2h. I tried to cancel it.
Have you tried to run the example in a local env with the 5.9.0 version of plotly?
Besides here is some feedback on the content of the PR itself.
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
|
Also it would be interesting to do a scatter plot of the result of the hyper-parameter search that compares mean test accuracy vs mean score duration. This can be done with plotly as in https://nbviewer.org/github/ogrisel/notebooks/blob/master/sklearn_demos/ames_housing.ipynb |
Co-authored-by: Olivier Grisel <[email protected]>
jjerphan
left a comment
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.
Thank you for this qualitative improvement, @ArturoAmorQ!
Adding plotly as a doc dependency works for me.
We need to make sure that HTTP links for the previous example (examples/model_selection/grid_search_text_feature_extraction.py) redirect to the new example (examples/model_selection/plot_grid_search_text_feature_extraction.py).
Apart from that, here are minor comments.
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
glemaitre
left a comment
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.
Some small changes.
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
examples/model_selection/plot_grid_search_text_feature_extraction.py
Outdated
Show resolved
Hide resolved
We should have something done for that in the |
|
To redirect the examples, it is in # redirects dictionary maps from old links to new links
redirects = {
"documentation": "index",
"auto_examples/feature_selection/plot_permutation_test_for_classification": (
"auto_examples/model_selection/plot_permutation_tests_for_classification"
),
"modules/model_persistence": "model_persistence",
"auto_examples/linear_model/plot_bayesian_ridge": (
"auto_examples/linear_model/plot_ard"
),
}
html_context["redirects"] = redirects
for old_link in redirects:
html_additional_pages[old_link] = "redirects.html" |
Co-authored-by: Julien Jerphanion <[email protected]>
Co-authored-by: Guillaume Lemaitre <[email protected]>
Co-authored-by: Guillaume Lemaitre <[email protected]>
Co-authored-by: Guillaume Lemaitre <[email protected]>
Co-authored-by: Guillaume Lemaitre <[email protected]>
jjerphan
left a comment
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.
LGTM. Thank you, @ArturoAmorQ.
As always, thank you all for your comments! |
glemaitre
left a comment
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.
Since it was the only change, I directly pushed it and I am going to merge. Thanks @ArturoAmorQ
Reference Issues/PRs
Related to #22928.
What does this implement/fix? Explain your changes.
Yet another release of the revamped examples on text analysis. This is a complement to the plot_document_classification_20newsgroups.py example, where a
GridSearchCVis used for text classification but is not shown to keep the example simple.Any other comments?
Side effect: Implements notebook style as intended in #22406.
Adds
plotlyto DOC dependencies.