Rewriting of the example code (plot_lle_digits.py) to generate again the broken images#22165
Conversation
|
Excuse me for all these commits, I got confused because I had made commits on the main directly for another issue. I only changed the path of the image for this issue. |
thomasjpfan
left a comment
There was a problem hiding this comment.
Thank you for the PR @cathelineparisot
doc/modules/manifold.rst
Outdated
| space and the similarities/dissimilarities. | ||
|
|
||
| .. figure:: ../auto_examples/manifold/images/sphx_glr_plot_lle_digits_010.png | ||
| .. figure:: ../auto_examples/manifold/images/sphx_glr_plot_lle_digits_002.png |
There was a problem hiding this comment.
I do not think this is enough to fix mainfold.rst because of all the missing images. I think we need to rework #20543 so that all the images are actually generated.
Basically #20543 lead to a regression in mainfold.rst. The previous version was better: https://scikit-learn.org/0.24/modules/manifold.html#multi-dimensional-scaling-mds
sklearn/pipeline.py
Outdated
| "or be the string 'passthrough' " | ||
| "'%s' (type %s) doesn't" % (t, type(t)) | ||
| ) | ||
| if (not (hasattr(t, "fit") or hasattr(t, "fit_transform")) or not |
There was a problem hiding this comment.
This is overlapping with another PR at #22162, we can focus on that there.
There was a problem hiding this comment.
I didn't do it on purpose, I handled Git wrong but I put back the original code.
…ques + modification conf
55f761f to
706bd41
Compare
77b9de9 to
e18a300
Compare
|
Thank you very much for your first feedback. I didn't understand what the problem was and now I've made the connection. Does it look right to you? The images are well generated again :) Also, I have this error: WARNING: Since version 2.0, Sphinx uses "index" as default root_doc. Please add "root_doc = 'contents'" to your conf.py. Should we add this to the Sphinx config? Thanks in advance for your feedback |
… to generate again the images
e18a300 to
724f719
Compare
No it is unrelated to the PR. If we want to update sphinx config, it should be done in another PR. There are a few things that needs to be done with this PR:
With all the points above, I do not think this is a great first issue. I recommend looking at #21350 for a good first issue to start getting use to scikit-learn's workflow. |
|
@cathelineparisot Would you be okay if another contributor continues this work? |
Hello, yes of course. I am no longer working on this issue so someone can take over. Thanks for asking ! |
Fixes #22061
What does this implement/fix? Explain your changes.
The image in the MDS document is not present
https://scikit-learn.org/stable/modules/manifold.html#multi-dimensional-scaling-mds
I changed the path of the image to something similar to the old version: https://scikit-learn.org/0.24/modules/manifold.html#multi-dimensional-scaling-mds
Any other comments?
Other images are not present in this same documentation page: only sphx_glr_plot_lle_digits_001.png and sphx_glr_plot_lle_digits_002.png exist in the image folder (doc/auto_example/manifold/images) but all others do not (3,4,5,6,7,8,9 and 13). This could be the occasion of another issue. I don't know what images to put instead. For my part, I changed the image 010 to 002.
Mine was generated from https://scikit-learn.org/dev/auto_examples/manifold/plot_lle_digits.html#sphx-glr-auto-examples-manifold-plot-lle-digits-py