Skip to content
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

[DOC]: Partially fix build error sphinx_gallery_conf not pickleable #28103

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

NirobNabil
Copy link

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

@github-actions github-actions bot added the Documentation: build building the docs label Apr 18, 2024
@tacaswell
Copy link
Member

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

@tacaswell
Copy link
Member

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:

@NirobNabil
Copy link
Author

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
Copy link
Member

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

@NirobNabil
Copy link
Author

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.

@@ -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
Copy link
Member

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.

Copy link
Member

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.

Copy link
Author

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

NirobNabil and others added 3 commits April 23, 2024 01:16
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
Copy link
Member

QuLogic commented May 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation: build building the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants