[DOC]: Fix compatibility with sphinx-gallery 0.16 #28103

NirobNabil · 2024-04-18T23:38:31Z

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 at 1289. 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 for subsection_order fixed but had to modify some code within sphinx_gallery. I will ask for the subsection_order fix in that sphinx_gallery PR. but other than that i was able to build the doc without the not pickleable warning (using the small change i made in sphinx_gallery code for subsection_order) following the conf.py changes introduced at that PR.

PR checklist

[N/A] "closes #0000" is in the body of the PR description to link the related issue
[N/A] new and changed code is tested
[N/A] Plotting related features are demonstrated in an example
[N/A] New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

tacaswell · 2024-04-19T00:35:59Z

You will probably also have to un-pin sphinx to see if this works.

tacaswell · 2024-04-19T00:46:14Z

Traceback (most recent call last):
  File "/home/circleci/.local/lib/python3.9/site-packages/sphinx_gallery/gen_gallery.py", line 261, in _fill_gallery_conf_defaults
    scraper = import_module(scraper)
  File "/home/circleci/.pyenv/versions/3.9.19/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 981, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'sphinxext.util.matplotlib_reduced_latex_scraper'; 'sphinxext.util' is not a package

No module named 'sphinxext.util.matplotlib_reduced_latex_scraper'; 'sphinxext.util' is not a package

Configuration error:
Unknown image scraper 'sphinxext.util.matplotlib_reduced_latex_scraper', got:
No module named 'sphinxext.util.matplotlib_reduced_latex_scraper'; 'sphinxext.util' is not a package

This looks like it is trying to import the function as a module (not import the (sub) module sphinxext.utils and then get a function out of it.

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:

unpin sphinx (undoing DOC: exclude sphinx 7.3.* #28094 )
install sphinx-gallery from the branch that PR is from

NirobNabil · 2024-04-19T09:30:59Z

I think if you want to test mpl with that pr you need to:

* unpin sphinx (undoing [DOC: exclude sphinx 7.3.* #28094](https://github.com/matplotlib/matplotlib/pull/28094) )

* install sphinx-gallery from the branch that PR is from

I did exactly that. Installed sphinx manually from the released source code at v7.3.6 and built sphinx_gallery by cloning from branch at larsoner's fork (which is the source of PR)

This is how i verified the versions and generated build logs

(matplotlib) nabil@pop-os:/media/nabil/workspace3/projects/gsoc24/matplotlib/doc$ python
Python 3.12.2 | packaged by Anaconda, Inc. | (main, Feb 27 2024, 17:35:02) [GCC 11.2.0] on linux
Type "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 > out 2>&1

The generated build logs is at out.txt

Note that i made some changes in the `sphinx_gallery` code

changed line 447 in gen_gallery to

sortkey = _get_class(gallery_conf, "subsection_order")

and changed the _get_class function in gen_rst at line 1503 to

if not inspect.isclass(val) and not hasattr(val, '__class__'):
        raise ConfigError(
            f"{what} must be 1) a fully qualified name (string) that resolves "
            f"to a class or 2) or a class itself 3) or an instance of a class,"
            f" got {val.__class__.__name__} ({repr(val)})"
        )

so that it passes with both a class definition or an instance of a class

tacaswell · 2024-04-19T13:58:04Z

Sorry, I meant to do that in the CI configuration so that we can see CI run clean!

NirobNabil · 2024-04-20T09:20:35Z

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.

tacaswell · 2024-04-22T01:46:51Z

.circleci/config.yml

@@ -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


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.

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.

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, so update that to match the new version. However, since that change is only a single period, I did not bother to keep the old version around.

QuLogic · 2024-05-02T17:06:57Z

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.

QuLogic · 2024-05-02T19:57:26Z

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.

…llery 0.16

lumberbot-app · 2024-05-08T21:08:58Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v3.8.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 a4243d9557da23d9a409b28695394704cf3cd6c9

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #28103: [DOC]: Fix compatibility with sphinx-gallery 0.16'

Push to a named branch:

git push YOURFORK v3.8.x:auto-backport-of-pr-28103-on-v3.8.x

Create a PR against branch v3.8.x, I would have named this PR:

"Backport PR #28103 on branch v3.8.x ([DOC]: Fix compatibility with sphinx-gallery 0.16)"

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 Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

lumberbot-app · 2024-05-08T21:09:19Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v3.8.4-doc
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 a4243d9557da23d9a409b28695394704cf3cd6c9

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #28103: [DOC]: Fix compatibility with sphinx-gallery 0.16'

Push to a named branch:

git push YOURFORK v3.8.4-doc:auto-backport-of-pr-28103-on-v3.8.4-doc

Create a PR against branch v3.8.4-doc, I would have named this PR:

"Backport PR #28103 on branch v3.8.4-doc ([DOC]: Fix compatibility with sphinx-gallery 0.16)"

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 Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

ksunden · 2024-05-08T21:10:09Z

Not going to worry about a manual backport for now

ksunden · 2024-05-08T21:11:01Z

@NirobNabil Thanks for this, congrats on your first PR merged, hope to hear from you again!

NirobNabil · 2024-05-09T15:08:23Z

@ksunden Sorry i was inactive for some days. Thanks for fixing it up and the merge and will surely be contributing more.
I also had another PR waiting for review for 2 months. Any update on that too will be really appreciated.

story645 · 2024-05-09T15:22:04Z

Any update on that too will be really appreciated.

😬 sorry about that, feel free to ping me privately if I owe you a review.

…103-on-v3.9.x Backport PR #28103 on branch v3.9.x ([DOC]: Fix compatibility with sphinx-gallery 0.16)

FIX (partial): sphinx_gallery_conf not pickleable

ccb5af5

github-actions bot added the Documentation: build building the docs label Apr 18, 2024

NirobNabil mentioned this pull request Apr 19, 2024

BUG: Fix serialization with Sphinx 7.3 sphinx-gallery/sphinx-gallery#1289

Merged

NirobNabil added 3 commits April 20, 2024 02:09

modify ci config to test updated sphinx_gallery

883535d

clean code & remove sphinx version restriction

f331594

remove unnecessary comment

ba8345f

properly undo matplotlib#28094

8c18b4d

tacaswell reviewed Apr 22, 2024

View reviewed changes

NirobNabil and others added 3 commits April 23, 2024 01:16

move sphinx-gallery installation back to requirements.txt~

6046659

DOC: Keep supporting older sphinx-gallery

4a80339

Update header patch for sphinx-gallery 0.16.0

bda6ac5

There was a small change to the `EXAMPLE_HEADER` that we patch, so update that to match the new version. However, since that change is only a single period, I did not bother to keep the old version around.

QuLogic marked this pull request as ready for review May 2, 2024 19:57

QuLogic added this to the v3.8-doc milestone May 2, 2024

rcomer mentioned this pull request May 5, 2024

FIX: update doc EXAMPLE_HEADER to match sphinx-gallery #28173

Closed

5 tasks

QuLogic changed the title ~~[DOC]: Partially fix build error sphinx_gallery_conf not pickleable~~ [DOC]: Fix compatibility with sphinx-gallery 0.16 May 8, 2024

ksunden approved these changes May 8, 2024

View reviewed changes

ksunden merged commit a4243d9 into matplotlib:main May 8, 2024
43 of 44 checks passed

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request May 8, 2024

Backport PR matplotlib#28103: [DOC]: Fix compatibility with sphinx-ga…

29e77e3

…llery 0.16

meeseeksmachine mentioned this pull request May 8, 2024

Backport PR #28103 on branch v3.9.x ([DOC]: Fix compatibility with sphinx-gallery 0.16) #28190

Merged

lumberbot-app bot added the Still Needs Manual Backport label May 8, 2024

ksunden removed this from the v3.8-doc milestone May 8, 2024

ksunden added this to the v3.9.0 milestone May 8, 2024

QuLogic removed the Still Needs Manual Backport label May 8, 2024

QuLogic added a commit that referenced this pull request May 9, 2024

Merge pull request #28190 from meeseeksmachine/auto-backport-of-pr-28…

2564db4

…103-on-v3.9.x Backport PR #28103 on branch v3.9.x ([DOC]: Fix compatibility with sphinx-gallery 0.16)

rossbar mentioned this pull request Jul 4, 2024

Update sphinx gallery config to enable sphinx build caching networkx/networkx#7548

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DOC]: Fix compatibility with sphinx-gallery 0.16 #28103

[DOC]: Fix compatibility with sphinx-gallery 0.16 #28103

NirobNabil commented Apr 18, 2024

tacaswell commented Apr 19, 2024

tacaswell commented Apr 19, 2024

NirobNabil commented Apr 19, 2024

tacaswell commented Apr 19, 2024

NirobNabil commented Apr 20, 2024

tacaswell Apr 22, 2024

timhoffm Apr 22, 2024

NirobNabil Apr 22, 2024

QuLogic commented May 2, 2024

QuLogic commented May 2, 2024

lumberbot-app bot commented May 8, 2024

lumberbot-app bot commented May 8, 2024

ksunden commented May 8, 2024

ksunden commented May 8, 2024

NirobNabil commented May 9, 2024

story645 commented May 9, 2024

[DOC]: Fix compatibility with sphinx-gallery 0.16 #28103

[DOC]: Fix compatibility with sphinx-gallery 0.16 #28103

Conversation

NirobNabil commented Apr 18, 2024

PR summary

PR checklist

tacaswell commented Apr 19, 2024

tacaswell commented Apr 19, 2024

NirobNabil commented Apr 19, 2024

Note that i made some changes in the sphinx_gallery code

tacaswell commented Apr 19, 2024

NirobNabil commented Apr 20, 2024

tacaswell Apr 22, 2024

Choose a reason for hiding this comment

timhoffm Apr 22, 2024

Choose a reason for hiding this comment

NirobNabil Apr 22, 2024

Choose a reason for hiding this comment

QuLogic commented May 2, 2024

QuLogic commented May 2, 2024

lumberbot-app bot commented May 8, 2024

lumberbot-app bot commented May 8, 2024

ksunden commented May 8, 2024

ksunden commented May 8, 2024

NirobNabil commented May 9, 2024

story645 commented May 9, 2024

Note that i made some changes in the `sphinx_gallery` code