Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
[DOC]: Fix compatibility with sphinx-gallery 0.16#28103
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
You will probably also have to un-pin sphinx to see if this works. |
This looks like it is trying to import the function as a module (not import the (sub) module In the removed code in that PR it looks like it used to always look for a particular name:https://github.com/sphinx-gallery/sphinx-gallery/pull/1289/files#diff-016e2fd897b8720412d190b7936fb295e31dc953c73fcb912ca1362ec8ca2191L261-L264 I think if you want to test mpl with that pr you need to:
|
I did exactly that. Installed sphinx manually from thereleased source code at v7.3.6 and built sphinx_gallery by cloning from branch atlarsoner's fork (which is the source ofPR) This is how i verified the versions and generated build logs (matplotlib) nabil@pop-os:/media/nabil/workspace3/projects/gsoc24/matplotlib/doc$ pythonPython 3.12.2| packaged by Anaconda, Inc.| (main, Feb 27 2024, 17:35:02) [GCC 11.2.0] on linuxType"help","copyright","credits" or"license"for more information.>>> import sphinx, sphinx_gallery>>> sphinx.__version__'7.3.6'>>> sphinx_gallery.__path__['/media/nabil/workspace3/projects/gsoc24/sphinx-gallery/sphinx_gallery']>>>exit()(matplotlib) nabil@pop-os:/media/nabil/workspace3/projects/gsoc24/matplotlib/doc$ make html> out2>&1 The generated build logs is atout.txt Note that i made some changes in the |
Sorry, I meant to do that in the CI configuration so that we can see CI run clean! |
I think documentation is building alright in current configuration.@tacaswell can you please check if its alright. I also couldn't figure out whats wrong with the two failed pytest steps or if these are even relevant. |
.circleci/config.yml Outdated
@@ -107,6 +107,7 @@ commands: | |||
python -m pip install --user \ | |||
numpy<< parameters.numpy_version >> \ | |||
-r requirements/doc/doc-requirements.txt | |||
pip install git+https://github.com/larsoner/sphinx-gallery.git@serial |
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.
You can put this detail is the requirements files (which means it will hit all of the CI that pull it and any developers who try to build locally will also get it.
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.
On a general note: It's ok to use a non-released version for developing this fix. But this should not go into our main branch. We should wait with merging until an update of sphinx-gallery is released, and if necessary pin sphinx in the mean 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.
You can put this detail is the requirements files (which means it will hit all of the CI that pull it and any developers who try to build locally will also get it.
moved sphinx-gallery installation to doc-requirements and all the checks passed :D
There was a small change to the `EXAMPLE_HEADER` that we patch, soupdate that to match the new version. However, since that change is onlya single period, I did not bother to keep the old version around.
I was looking into this since sphinx-gallery 0.16.0 is out now, and started working on it before I found this PR. The older version doesn't support using the strings AFAICT, so I'm going to push a change here that works for both. |
I'm going to mark this as ready; it looks good to me, though I'm biased since I made some of the changes here. |
a4243d9
intomatplotlib:mainUh oh!
There was an error while loading.Please reload this page.
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free tosuggest an improvement. |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free tosuggest an improvement. |
Not going to worry about a manual backport for now |
@NirobNabil Thanks for this, congrats on your first PR merged, hope to hear from you again! |
@ksunden Sorry i was inactive for some days. Thanks for fixing it up and the merge and will surely be contributing more. |
😬 sorry about that, feel free to ping me privately if I owe you a review. |
…103-on-v3.9.xBackport PR#28103 on branch v3.9.x ([DOC]: Fix compatibility with sphinx-gallery 0.16)
PR summary
I was following a thread on the same issue in sphinx-gallery. It seems every callables and classes need to be stringified. There is some work being done on sphinx-gallery side at1289. But even with this PR the build fails, My assumption is that its because of the callable in
subsection_order
key which is not being handled on sphinx_gallery side. I was able to get the error forsubsection_order
fixed but had to modify some code within sphinx_gallery. I will ask for thesubsection_order
fix in thatsphinx_gallery PR. but other than that i was able to build the doc without thenot pickleable warning (using the small change i made in sphinx_gallery code for subsection_order) following the conf.py changes introduced atthat PR.PR checklist