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

Appearance settings

Early check for dot binary (from graphviz) when building the doc (fixes #8207) #8221

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

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

afvincent
Copy link
Contributor

Fixes #8207.

The commit is shamelessly inspired from #8213.

@afvincent afvincent added this to the 2.0.1 (next bug fix release) milestone Mar 7, 2017
Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good if tests pass!

@NelleV NelleV changed the title Early check for dot binary (from graphviz) when building the doc (fixes #8207) [MRG+1] Early check for dot binary (from graphviz) when building the doc (fixes #8207) Mar 7, 2017
@NelleV
Copy link
Member

NelleV commented Mar 7, 2017

It also look good to me, though the amount of dependencies we have in our documentation worries me. It means that new contributors struggle with something as simple as contributing documentation.

@NelleV NelleV changed the title [MRG+1] Early check for dot binary (from graphviz) when building the doc (fixes #8207) [MRG+2] Early check for dot binary (from graphviz) when building the doc (fixes #8207) Mar 7, 2017
@anntzer
Copy link
Contributor

anntzer commented Mar 7, 2017

@NelleV I wonder whether we could create two mpl-testing and mpl-docs packages on conda-forge which would pull in all required dependencies, including the non-python ones (for example, graphviz is packaged by conda). Then the instructions would just be to install the corresponding package from conda.

@NelleV
Copy link
Member

NelleV commented Mar 8, 2017

@anntzer I don't use conda, so I have no opinion on whether this would be useful.
In general, I do not think it is wise to rely too much on conda for a project as wide as Matplotlib: Matplotliib should be able to get contributors from non scientific python users, and conda is not widely used outside of the Python community.

An option would be to change the default for allowsphinxwarnings so that people can build the documentation with missing dependencies. That would reduce the barrier for contributions, but then it would be up to us (core contributors) to guide new contributors on fixing travis build. I do not think we are well equipped for that option.

@anntzer
Copy link
Contributor

anntzer commented Mar 8, 2017

Alternatively, I know that imageio has been including code to download ffmpeg/avbin/freeimage at runtime (http://imageio.readthedocs.io/en/latest/envvariables.html), so that could be an option too (although it would certainly have its own issues, unless they have completely statically linked builds...).

@NelleV NelleV merged commit bb167dd into matplotlib:master Mar 8, 2017
@NelleV
Copy link
Member

NelleV commented Mar 8, 2017

Thanks @afvincent !

@afvincent afvincent deleted the doc_early_check_for_dot branch March 8, 2017 14:44
@QuLogic QuLogic changed the title [MRG+2] Early check for dot binary (from graphviz) when building the doc (fixes #8207) Early check for dot binary (from graphviz) when building the doc (fixes #8207) Mar 20, 2017
dstansby pushed a commit that referenced this pull request Mar 25, 2017
Early check for dot binary (from graphviz) when building the doc (fixes #8207)
@dstansby
Copy link
Member

Backported to 2.0.x via. c8b4db8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.