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
6.1.2 breaks the aafigure extension, it worked in 6.1.1 #11117
Comments
Matplotlib's sphinx extension is also similarly affected (not sure yet if it is the same cause, we are getting assertion errors in our test suite rather than direct errors raised by sphinx, but it is new with 6.1.2) Ours does seem to be affected by #11100, which causes additional |
Okay, my digging has shown that #11100 changed from using
For aafigure, this means trying to copy a folder by opening it as a file, which is an PermissionError. For matplotlib, this means that a py file gets copied where it hadn't been before and our tests catch that change. Arguably, the proper fix is to prevent unsupported image types from entering The logic around |
The change to .env.images was done to allow more parallelism. If there are changes Sphinx should make here, would you be able to open a PR? I may have time to review it but due to other commitments I won't be able to look at this myself before May-ish. A |
Sphinx 6.1.2 introduces new behavior which causes python source files to be copied into the images folder errantly. For a more full write up refer to sphinx-doc/sphinx#11117
The following patch (on 6.1.x) appears to fix the diff --git a/sphinx/environment/collectors/asset.py b/sphinx/environment/collectors/asset.py
index 1ab7c09de..cb7ec9a34 100644
--- a/sphinx/environment/collectors/asset.py
+++ b/sphinx/environment/collectors/asset.py
@@ -45,6 +45,13 @@ class ImageCollector(EnvironmentCollector):
candidates: dict[str, str] = {}
node['candidates'] = candidates
imguri = node['uri']
+ if imguri == "":
+ # image nodes created (via directive from extensions) via
+ # docutils.parsers.directives.images.Image.run()
+ # may create node of the type <image uri=""/>. The image
+ # files do not exist yet in source, let's skip.
+ candidates['?'] = imguri
+ continue
if imguri.startswith('data:'):
candidates['?'] = imguri
continue (the comments I added are for my own education and not quite precise; thanks @ksunden for your analysis, I have not checked the PS @ksunden do you happen to have a small project showing the issue with |
I have released v6.1.3 which should fix this by reverting the parallel image changes. A |
https://gist.github.com/ksunden/1b726225cbc93e65c7e478fda4dd5a1f Here is a gist with a fairly minimal conf/Makefile extracted from our tests (still using the mpl extension, so not minimal in that dimension, but outside of that it is fairly minimal) (the makefile was edited to create an empty It builds without warning/error, but the I will note that mpl's sphinx extension was recently modified in matplotlib/matplotlib#24205 and undoing that change does actually fix our tests, but leaves many files in |
I can confirm that v6.1.3 with the reverted changes fixes the aafigure issue for me. |
Thanks @ksunden for very small example indeed (I used the bare sphinx-quickstart Makefile). I had not seen the issue when trying directly earlier because I was using Matplotlib The With
$ find _build/html -name '*.py'
_build/html/_downloads/25f419794f3589105a3cedf01038cfc8/index-1.py
$ find _build/html -name '*.py'
_build/html/_downloads/25f419794f3589105a3cedf01038cfc8/index-1.py
_build/html/_images/index-1.py
$ find _build/html -name '*.py'
_build/html/_downloads/25f419794f3589105a3cedf01038cfc8/index-1.py With matplotlib
$ find _build/html -name '*.py'
_build/html/index-1.py The In summary it seems users of Matplotlib will not be hit by this Sphinx |
Describe the bug
The latest version of sphinx, 6.1.2, breaks the aafigure extension, it makes it try to copy the src folder instead of the actual images I think.
How to Reproduce
index.rst with empty aafigure directive + conf.py with the extension is enough to trigger the error:
Environment Information
Sphinx extensions
["aafigure.sphinxext"]
Additional context
I wonder if its this PR that causes it? #11100
The text was updated successfully, but these errors were encountered: