ipython/ipython

`ipython_directive` @savefig: should detect missing static html dir and not place output in source dir

stefanv opened this issue · 28 comments

  1. The ipython_directive's @savefig currently has its default output set relative to the static html directory. If that directory is missing, it places the output outside of the source tree. The build succeeds with a warning from Sphinx that it cannot find the image—which does not help to find the source of the problem. Solution: raise an error/warning if the directive's output path is invalid.
  2. @savefig stores its output in the source directory. A better default would be to store it in build/_static.
minrk commented

@stefanv can you test #8888? I think it fixes these things, but I'm not super experienced with this stuff.

That does not seem to work for me, but it may very well be that I am not testing correctly. The repository I am testing against is

https://github.com/statlab/permute

Where I see:

internal padding after /home/stefan/akad/projekte/permute/doc/source/user/npc/kenya.rst:30: WARNING: image file not readable: user/npc/../../../build/html/_static/test.png

(after commenting out ipython_savefig_dir from the config)

minrk commented

Thanks for testing, I'll use that repo to figure out what I'm not quite doing correctly.

@stefanv I do not seem to be able to reproduce using permute and coomenting the ipython_savefig_dir could sphinx-build have picked up a ipython savefig directive from another env ?

 git diff -U0
diff --git a/doc/source/conf.py b/doc/source/conf.py
index 4fb1ba1..a6bff13 100644
--- a/doc/source/conf.py
+++ b/doc/source/conf.py
@@ -147 +147 @@ html_static_path = ['_static']
-ipython_savefig_dir = '../build/html/_static'
+# ipython_savefig_dir = '../build/html/_static'
$ make html
sphinx-build -b html -d build/doctrees   source build/html
Running Sphinx v1.3.3
making output directory...
loading pickled environment... not yet created
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 29 source files that are out of date
updating environment: 29 added, 0 changed, 0 removed
checking for /Users/bussonniermatthias/dev/permute/doc/source/permute.bib in bibtex cache... not found
parsing bibtex file /Users/bussonniermatthias/dev/permute/doc/source/permute.bib... parsed 10 entries
reading sources... [100%] user/two-sample
/Users/bussonniermatthias/dev/permute/permute/core.py:docstring of permute.core.one_sample:37: ERROR: Undefined substitution referenced: "z_i".
/Users/bussonniermatthias/dev/permute/permute/core.py:docstring of permute.core.two_sample_conf_int:30: ERROR: Undefined substitution referenced: "F_x(t) - F_y(t)".
/Users/bussonniermatthias/dev/permute/permute/core.py:docstring of permute.core.two_sample_shift:33: ERROR: Undefined substitution referenced: "F_x(t) - F_y(t)".
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] user/two-sample
generating indices... genindex py-modindex
highlighting module code... [100%] permute.stratified
writing additional pages... search
copying images... [100%] user/npc/../../../build/plot_directive/user/npc/kenya-1.png
copying static files... done
copying extra files... done
dumping search index in English (code: en) ... done
dumping object inventory... done
build succeeded, 3 warnings.

Build finished. The HTML pages are in build/html.

Note that I needed to work around matplotlib/matplotlib#5884 while building.

Note that one now has to roll permute back to an earlier commit to test this, because the example @stefanv referenced has been switched to use a matplotlib directive.

I also couldn't reproduce this. I'm dropping it from the 4.1 milestone because there's nothing we can really do until someone can reproduce it.

Sounds reasonable. We can always reopen it if we do. Thanks Stefan.

I think I'm running into this issue on 4.0.1 and a related issue on 4.1.1

Sorry in advance for not putting together a minimum reproducible example and pointing you to my mess of code...

I'm trying to build the documentation for this project:

https://github.com/wholmgren/pvlib-python/tree/pvsystem

The figures on this rtd page will appear if I use IPython 4.0.1, but they will not appear using IPython 4.1.1.

I think this is because on 4.0.1, IPython puts the figures in my source/_static if the build directory does not exist, and then they eventually get copied over to the right spot. So it's not ideal, as described above, but at least it works. However, on 4.1.1, IPython puts the figures in the directory from which I'm running make html and they never make it to the right place.

Reopening to have look.

I am having the exact same issue as well using Sphinx v1.3.1, and IPython 4.1.1. I created a minimal working example to demonstrate the issue here:

https://github.com/karthik1024/trialproject

When I try to build documentation , I get a warning as shown below and the saved figure does not appear in the documentation:

WARNING: image file not readable: ..\doc\build\html\_static\saved_figure.png

where saved_figure.png is the image that is created using @savefig..

Having the same issue with the geopandas docs (geopandas/geopandas#298 (comment)). So actually ipython 4.1 is broken for the savefig feature, and I have to pin ipython on 4.0

I'm having the same issues with 4.2 and 4.1, needed to downgrade to 4.0.

And just a heads up in case anyone runs into the same problem:

I was getting a lot internal padding after some_file.rst WARNING: image file not readable: some_image.png because of the images being misplaced, after downgrading to 4.0 I was still getting some of the same errors, I later realized that it was because of an error syntax in a python statement (a closing parenthesis was missing).

FYI, this sadly remains broken in 5.0rc1.

moved from 'no action' to 5.1, but have we have no clue why or how to reproduce it's hard to investigate. We'll re-have a look .

saimn commented

Seems very similar to #8969 : what I found is that, if the output directory does not exists, which is the case for a clean build, bookmark ipy_savedir fails and image are saved in a wrong directory. I fixed that in my project by forcing the creation of the output directory.

This is also affecting the xarray doc build since we updated our read-the-docs builds to ipython 5 and python 3 (RTD logs). I've been able to solve the problem locally with the following add to conf.py:

savefig_dir = os.path.join('_build','html','_static')
if not os.path.exists(savefig_dir):
    os.makedirs(savefig_dir)

For some reason this doesn't work on RTD (probably because their build directory structure is different). I'd appreciate any hint on how to get this working.

Small update: we have now pinned ipython to 4.0.3

Now the docs are at least building as expected and the images are shown. This is only partially working, as the readthedocs builds end with an error (which doesn't make the build fail):

(...)
build succeeded, 26 warnings.
Error in atexit._run_exitfuncs:
Traceback (most recent call last):
  File "/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/latest/lib/python3.5/site-packages/IPython/core/history.py", line 547, in end_session
    len(self.input_hist_parsed)-1, self.session_number))
sqlite3.OperationalError: attempt to write a readonly database

This error doesn't happen with ipython 5, but again with ipython 5 we get the image file not readable error, which is worse.

Let me know if I can provide you with more info, as I am not sure to understand the problem very well myself...

This is still in need of someone to dig into it and work it out. We don't actually use the ipython_directive for IPython's own docs, so we don't have much motivation to prioritise it ourselves.

@takluyver Perhaps a widgets issue. Here's a failing build from rtd and xarray for reference when others troubleshoot this: http://readthedocs.org/projects/xray/builds/4546462/

Thanks for the info. Here is a build with ipython 5.1 : https://readthedocs.org/projects/xray/builds/4760089/ The build succeeds but the images are not copied correctly, very likely because of what @saimn said above. We are pinned on 4.0.3 since then, which currently suits our needs.

I think the widgets warning is unrelated to this error.

Can anyone who's having trouble with this try #10183?

Thanks @takluyver !

I think this is on the right track, but unfortunately RTD seems unhappy with this: https://readthedocs.org/projects/xray/builds/4916490/

Partial traceback:

(...)
 File "/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/fix-docs/lib/python3.5/site-packages/IPython/sphinxext/ipython_directive.py", line 930, in run
   rgxin, rgxout, promptin, promptout = self.setup()
 File "/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/fix-docs/lib/python3.5/site-packages/IPython/sphinxext/ipython_directive.py", line 898, in setup
   os.makedirs(os.path.abspath(savefig_dir))
 File "/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/fix-docs/lib/python3.5/os.py", line 241, in makedirs
   mkdir(name, mode)
OSError: [Errno 30] Read-only file system: '/home/docs/checkouts/readthedocs.org/readthedocs/templates/sphinx/_static'

Exception occurred:
 File "/home/docs/checkouts/readthedocs.org/user_builds/xray/conda/fix-docs/lib/python3.5/os.py", line 241, in makedirs
   mkdir(name, mode)
OSError: [Errno 30] Read-only file system: '/home/docs/checkouts/readthedocs.org/readthedocs/templates/sphinx/_static'

I'm not familiar enough with RTD and ipython to understand whats going on here. Also, I wonder what changed between ipython4 and ipython5 so that this change would be necessary :(

Looking at the error and the code, it seems to think that outdir is a template directory, for some reason. i'm rather puzzled by that. RTD, not unreasonably, appears to put that in a read-only location, so trying to create the directory there fails.

I think your config here is being ignored, by the way, because the option appears to be called ipython_savefig_dir rather than savefig_dir.

There were some changes which went into 4.1 (#8888). Since you're pinned to 4.0.3, maybe that's what broke it for you?

There were some changes which went into 4.1 (#8888). Since you're pinned to 4.0.3, maybe that's what broke it for you?

Yes, if I recall well I tried several options and 4.0.3 was the last one to work. Now I've tried a new build with your suggested changes modified to:

# Ensure that the directory for saving figures exists
try:
    os.makedirs(os.path.abspath(savefig_dir))
except:
    pass

(fmaussion@4e49fbd)

The build completes, but as before the images just won't show up... (log: https://readthedocs.org/projects/xray/builds/4917004/)

I don't really get it, do we do anything wrong here? :-( (btw, I also just removed the obsolete option you mentioned in our conf.py to do the test...)

Your change silences the error creating the directory, but if it can't create the directory, it won't be able to save images into it, so it's not fixing the underlying issue.

We calculate the savefig directory relative to the output directory:
https://github.com/takluyver/ipython/blob/4dc7caa9ede16968301d3abe5f031120d4de00d6/IPython/sphinxext/ipython_directive.py#L853

What I don't understand is that your error indicates that the output directory is /home/docs/checkouts/readthedocs.org/readthedocs/templates/sphinx/, which doesn't make much sense.

Thanks @takluyver , this is very helpful. I've been able to solve the problem by setting manually the ipython_savefig_dir option in conf.py:

ipython_savefig_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)),
'_build','html','_static')

I still think something should be solved upstream, as this option should be optional.

Using https://github.com/karthik1024/trialproject , I've resolved the issue with images not showing up. But you still see the 'image file not readable' warning, because it expects to copy images from the source directory to the build directory. I'm not sure how to work around that other than writing the images to the source directory (i.e. reverting part of #8888).

@minrk - any thoughts? As you wrote #8888.

I've made #10186 as an alternative fix, if anyone wants to try it. It moves images back into the docs source directory, which gets rid of the warnings.