-
-
Notifications
You must be signed in to change notification settings - Fork 26.2k
MNT Accelerate_examples plot_digits_linkage.py #21737
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
…for plot_digits_linkage.py
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.
Otherwise LGTM.
@@ -2,35 +2,28 @@ | |||
============================================================================= | |||
Various Agglomerative Clustering on a 2D embedding of digits | |||
============================================================================= | |||
|
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.
Please revert all the changes in the first docstring. They are not linked with the purpose of the PR and this is fine to have paragraph.
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.
Happy to merge once there are reverted
""" | ||
|
||
# Authors: Gael Varoquaux | ||
# License: BSD 3 clause (C) INRIA 2014 | ||
|
||
from time import time |
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.
Let's not introduce perf_counter
that is never used in the documentation and stick to time
.
clustering.fit(X_red) | ||
print("%s :\t%.2fs" % (linkage, time() - t0)) | ||
print("%s :\t%.2fs" % (linkage, perf_counter() - t0)) |
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.
However, here we can use an f-string and the same in the title in plot_clustering
below.
str(y[i]), | ||
color=plt.cm.nipy_spectral(labels[i] / 10.0), | ||
fontdict={"weight": "bold", "size": 9}, | ||
for t in np.sort(np.unique(y)): |
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.
Instead of make the sorting we can load earlier the full Bunch:
digits = datasets.load_digits()
X, y = digits.data, digits.target
and then use the target_names
that will be the array that you are expecting with the unique + sort.
for t in np.sort(np.unique(y)): | |
for digit in digits.target_names: |
*X_red[y == t].T, | ||
marker=f"${t}$", | ||
s=50, | ||
c=plt.cm.nipy_spectral(labels[y == t] / 10), |
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.
c=plt.cm.nipy_spectral(labels[y == t] / 10), | |
c=plt.cm.nipy_spectral(labels[y == digit] / 10), |
…g back perf_counter to time
Closing since the other PR was opened before this one. Please check if there's an open PR before working on an example @siavrez |
Sorry, didn't see that we had already merged that one. |
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.
Thanks @siavrez
An illustration of various linkage option for agglomerative clustering on | ||
a 2D embedding of the digits dataset. | ||
|
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.
We only to revert these small changes here and we can merge this one.
Thanks @siavrez |
Changing matplotlib.text with matplotlib.scatter and Markers created from TeX symbols improves runtime by almost 15 seconds.
Also changed time.time with time.perf_counter.
Reference Issues/PRs
#21598
What does this implement/fix? Explain your changes.
Changing matplotlib.text with matplotlib.scatter and Markers created from TeX symbols improves runtime by almost 15 seconds.
Also changed time.time with time.perf_counter.
Any other comments?