Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
ksunden merged 8 commits intomatplotlib:mainfromNirobNabil:serialize
May 8, 2024

Conversation

NirobNabil
Copy link
Contributor

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

@github-actionsgithub-actionsbot added the Documentation: buildbuilding the docs labelApr 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_unlockedModuleNotFoundError: No module named 'sphinxext.util.matplotlib_reduced_latex_scraper'; 'sphinxext.util' is not a packageNo module named 'sphinxext.util.matplotlib_reduced_latex_scraper'; 'sphinxext.util' is not a packageConfiguration 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) modulesphinxext.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
ContributorAuthor

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 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 thesphinx_gallery code

  • changedline 447 in gen_gallery to

    sortkey=_get_class(gallery_conf,"subsection_order")
  • and changed the_get_class function in gen_rst atline 1503 to

    ifnotinspect.isclass(val)andnothasattr(val,'__class__'):raiseConfigError(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
ContributorAuthor

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.

NirobNabil reacted with thumbs up emoji
Copy link
ContributorAuthor

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

NirobNabiland others added3 commitsApril 23, 2024 01:16
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.
@QuLogic
Copy link
Member

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

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.

@QuLogicQuLogic marked this pull request as ready for reviewMay 2, 2024 19:57
@QuLogicQuLogic added this to thev3.8-doc milestoneMay 2, 2024
@QuLogicQuLogic changed the title[DOC]: Partially fix build error sphinx_gallery_conf not pickleable[DOC]: Fix compatibility with sphinx-gallery 0.16May 8, 2024
@ksundenksunden merged commita4243d9 intomatplotlib:mainMay 8, 2024
43 of 44 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestMay 8, 2024
@lumberbot-appLumberbot (App)
Copy link

Owee, I'm MrMeeseeks, Look at me.

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

  1. Checkout backport branch and update it.
git checkout v3.8.xgit pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 a4243d9557da23d9a409b28695394704cf3cd6c9
  1. 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'
  1. Push to a named branch:
git push YOURFORK v3.8.x:auto-backport-of-pr-28103-on-v3.8.x
  1. 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 theStill Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free tosuggest an improvement.

@lumberbot-appLumberbot (App)
Copy link

Owee, I'm MrMeeseeks, Look at me.

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

  1. Checkout backport branch and update it.
git checkout v3.8.4-docgit pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 a4243d9557da23d9a409b28695394704cf3cd6c9
  1. 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'
  1. Push to a named branch:
git push YOURFORK v3.8.4-doc:auto-backport-of-pr-28103-on-v3.8.4-doc
  1. 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 theStill Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free tosuggest an improvement.

@ksundenksunden removed this from thev3.8-doc milestoneMay 8, 2024
@ksunden
Copy link
Member

Not going to worry about a manual backport for now

@ksunden
Copy link
Member

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

@NirobNabil
Copy link
ContributorAuthor

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

@story645
Copy link
Member

Any update on that too will be really appreciated.

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

QuLogic added a commit that referenced this pull requestMay 9, 2024
…103-on-v3.9.xBackport PR#28103 on branch v3.9.x ([DOC]: Fix compatibility with sphinx-gallery 0.16)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell left review comments

@timhoffmtimhoffmtimhoffm left review comments

@ksundenksundenksunden approved these changes

Assignees
No one assigned
Labels
Documentation: buildbuilding the docs
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.

6 participants
@NirobNabil@tacaswell@QuLogic@ksunden@story645@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp