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

Stop make.py removing generated documentation figs #3253

Merged
merged 1 commit into from Jul 16, 2014

Conversation

dmcdougall
Copy link
Member

I think this fixes #2908. Links seem to work locally now.

@NelleV
Copy link
Member

NelleV commented Jul 14, 2014

We should probably update the sphinx layout and replace the make.py file with a "real" makefile at some point. I think that just copying the default sphinx makefile "just works".

@mdboom
Copy link
Member

mdboom commented Jul 14, 2014

Wow, @dmcdougall. Thanks for getting to the bottom of that. Ouch.

@mdboom mdboom added this to the v1.4.0 milestone Jul 14, 2014
@dmcdougall
Copy link
Member Author

@NelleV That would be great. It'd also mean we can get a parallel doc build out of the box. Right now the make.py file spawns a subprocess that invokes sphinx-build, and my effort to add a -j flag to that command didn't seem to work for me.

@mdboom Ouch indeed. I think removing these lines is ok, since everything is generated in the build directory anyway. The plot_directive function is copying all of the figures to the right destination in the build directory.

It'd be really helpful if someone could try out this PR as a sanity check and make sure a few of the broken links (listed in #2908) work as expected.

@dmcdougall
Copy link
Member Author

Also, the reason I didn't use the python make.py linkcheck function is that it appears to be the case that if links refer to a local document, sphinx assumes they're good. That is, sphinx only check external links. It might be an idea to try use the solution here to check for local link target. If that doesn't work for hyperlinks, perhaps making a feature request to sphinx upstream to force sphinx to follow local links and check they're not borked.

@dmcdougall
Copy link
Member Author

Here's the output from linkchecker:

$ linkchecker index.html
LinkChecker 9.2              Copyright (C) 2000-2014 Bastian Kleineidam
LinkChecker comes with ABSOLUTELY NO WARRANTY!
This is free software, and you are welcome to redistribute it
under certain conditions. Look at the file `LICENSE' within this
distribution.
Get the newest version at http://wummel.github.io/linkchecker/
Write comments and bugs to https://github.com/wummel/linkchecker/issues
Support this project at http://wummel.github.io/linkchecker/donations.html

Start checking at 2014-07-16 14:52:55-005
10 threads active,   921 links queued,  502 links in  28 URLs checked, runtime 1 seconds

URL        `Matplotlib.pdf'
Name       `PDF'
Parent URL file:///Users/damon/git/github/matplotlib/doc/build/html/contents.html, line 139, col 13
Real URL   file:///Users/damon/git/github/matplotlib/doc/build/html/Matplotlib.pdf
Check time 0.002 seconds
Result     Error: URLError: <urlopen error [Errno 2] No such file or directory: '/Users/damon/git/github/matplotlib/doc/build/html/Matplotlib.pdf'>
 9 threads active,  4188 links queued, 3312 links in  58 URLs checked, runtime 6 seconds
10 threads active,  4980 links queued, 7716 links in  93 URLs checked, runtime 11 seconds
 9 threads active,  5961 links queued, 11092 links in 121 URLs checked, runtime 16 seconds
 9 threads active,  6285 links queued, 15325 links in 143 URLs checked, runtime 21 seconds
10 threads active,  6770 links queued, 18653 links in 204 URLs checked, runtime 26 seconds
 9 threads active,  7489 links queued, 22218 links in 290 URLs checked, runtime 31 seconds
10 threads active,  7947 links queued, 25631 links in 404 URLs checked, runtime 36 seconds
10 threads active,  8344 links queued, 29316 links in 572 URLs checked, runtime 41 seconds
 9 threads active,  5117 links queued, 36168 links in 683 URLs checked, runtime 46 seconds
10 threads active,  5759 links queued, 39777 links in 839 URLs checked, runtime 51 seconds
 4 threads active,  3934 links queued, 44472 links in 2219 URLs checked, runtime 56 seconds

Statistics:
Downloaded: 17.5MB.
Content types: 3990 image, 36847 text, 0 video, 0 audio, 3231 application, 2 mail and 4365 other.
URL lengths: min=14, max=159, avg=79.

That's it. 48435 links in 5132 URLs checked. 0 warnings found. 1 error found.
Stopped checking at 2014-07-16 14:53:53-005 (57 seconds)

From 82 errors to one error, so I think this is probably good to merge.

@mdboom
Copy link
Member

mdboom commented Jul 16, 2014

:woot!:

mdboom added a commit that referenced this pull request Jul 16, 2014
Stop make.py removing generated documentation figs
@mdboom mdboom merged commit fef699a into matplotlib:v1.4.x Jul 16, 2014
@tacaswell
Copy link
Member

And that error makes sense because you have not also built the pdf.

@WeatherGod
Copy link
Member

Can we make linkchecker a part of the release procedure or maybe for
something a bit more automated?

On Wed, Jul 16, 2014 at 4:46 PM, Thomas A Caswell notifications@github.com
wrote:

And that error makes sense because you have not also built the pdf.


Reply to this email directly or view it on GitHub
#3253 (comment)
.

@dmcdougall
Copy link
Member Author

@tacaswell Correct, I didn't build the latex, only the html.

@WeatherGod LinkChecker is GPL'd; I know not of the consequences of shipping LinkChecker with matplotlib. Instead of shipping it, we could do some foo to detect if LinkChecker is installed and, if so, use it to test the docs. I'd like to see that get run in the Travis builds. Documentation only takes 10 minutes to build.

@mdboom
Copy link
Member

mdboom commented Jul 16, 2014

There is also a linkcheck command built into Sphinx that we could use.

@dmcdougall
Copy link
Member Author

@mdboom python make.py linkcheck doesn't check local links (see this comment).

@mdboom
Copy link
Member

mdboom commented Jul 16, 2014

Oh yeah -- I forgot I saw that fly by. I think we're fine licensing-wise to install LinkChecker on Travis and use it. That would get us most of the way there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants