DOC Fix notebook style of plot_coin_ward_segmentation #23164
Conversation
There was a problem hiding this comment.
A couple of comments to improve a bit the example.
Otherwise LGTM.
Thanks @lucyleeow
| # Generate data | ||
| # ------------- | ||
|
|
||
| orig_coins = coins() | ||
|
|
||
| # Resize it to 20% of the original size to speed up the processing |
There was a problem hiding this comment.
| # Resize it to 20% of the original size to speed up the processing | |
| # %% | |
| # Resize it to 20% of the original size to speed up the processing |
| @@ -27,8 +27,10 @@ | |||
| from sklearn.cluster import AgglomerativeClustering | |||
There was a problem hiding this comment.
Since we are modifying the example, you can move the import to the cell where it is used the first time.
| @@ -41,12 +43,18 @@ | |||
|
|
|||
There was a problem hiding this comment.
A FutureWarning is raised by rescale because of multichannel=False. Indeed, it was already the default so we can simply remove this keyword.
| # Plot the results on an image | ||
| # ---------------------------- | ||
|
|
||
| plt.figure(figsize=(5, 5)) | ||
| plt.imshow(rescaled_coins, cmap=plt.cm.gray) | ||
| for l in range(n_clusters): |
There was a problem hiding this comment.
instead of plot.xticks and plt.yticks, let use plt.axis("off") which is more meaningful.
| @@ -59,8 +67,10 @@ | |||
| print("Number of pixels: ", label.size) | |||
There was a problem hiding this comment.
Let's use some f-string in the different print statement and use {time.time() - st:.3f} s instead of showing too many digits
| # Plot the results on an image | ||
| # ---------------------------- | ||
|
|
There was a problem hiding this comment.
After showing the image, it could be nice to add a small sentence to describe the results. In short, we are able to segment the different coins but we need to provide n_clusters larger than the actual number of coins because the segmentation is finding a large cluster in the background.
|
Thanks for review @glemaitre ! Changes made |
|
The CI failures seem unrelated to this PR |
Yes, there is a PR to solve this problem. |
1 similar comment
Yes, there is a PR to solve this problem. |
|
I directly made the changes since the example looks fine otherwise. |
| @@ -9,44 +9,55 @@ | |||
|
|
|||
| """ | |||
|
|
|||
| # %% | |||
There was a problem hiding this comment.
This marker makes the rendering weird with some of the font being in bold.
You can either revert or make the rendering maybe with some RST items, as you prefer.
|
@lucyleeow Thanks merging. |
…3164) Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Reference Issues/PRs
#22406
What does this implement/fix? Explain your changes.
Fixes the notebook style of the example
Any other comments?