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

Warn the user to set the Agg backend #157

Merged
merged 3 commits into from Nov 8, 2016

Conversation

Projects
None yet
4 participants
@Titan-C
Member

Titan-C commented Oct 26, 2016

closes #153

This includes an additional warning message when loading the Agg backend if matplotlib has loaded any other backend before.

@lesteve

This comment has been minimized.

Contributor

lesteve commented Oct 26, 2016

Why not an error, i.e. is there a use case where keeping on going with a backend other than Agg is useful? I haven't tried but for an interactive backend I would expect the build to stop at the first plt.show() ...

@Titan-C

This comment has been minimized.

Member

Titan-C commented Oct 26, 2016

Why not an error, i.e. is there a use case where keeping on going with a backend other than Agg is useful? I haven't tried but for an interactive backend I would expect the build to stop at the first plt.show()

I thought of a warning because the only reason to use Agg is to avoid the pop up. The use case is none. Yes interactive backend will pop the figures on plt.show() and the user will have to close them by hand.
I'll make it a Value error

@Titan-C Titan-C force-pushed the Titan-C:matploback branch from fe775a0 to 46b1085 Oct 26, 2016

@GaelVaroquaux

This comment has been minimized.

Contributor

GaelVaroquaux commented Oct 26, 2016

@Titan-C

This comment has been minimized.

Member

Titan-C commented Oct 26, 2016

Why not an error, i.e. is there a use case where keeping on going with a
backend other than Agg is useful?

SVG would work.

I actually gave it a try. For now writing images in a different format than PNG is not really supported. Main immediate problem is thumbnails(we use pillow to resize them), I actually hacked it to just pass the svg as they don't really need to be resized. With PDF, web browsers can't display pdf images only as embed or opening them individual. Thus in both cases it does not look nice.

But my new question is Agg can generate PNG and SVG and EPS and PDF(And at some point we could support all those formats), why bother selecting the other non interactive backends?

I haven't tried but for an interactive backend I would expect the build
to stop at the first plt.show() ...

I think that we monkey-patch matplotlib to avoid this.

Monkey-patching seem unnecessary hard. I tried loading matplotib in interactive mode. And although it works well in the Python/IPython interpreter to continue execution after plt.show() is called. It doesn't when running sphinx-build and the figures open in my display, it is not until I close all of them that the build can continue with the next example. But most important (except the seaborn example) once I close the image there are no figure objects to save, thus no figure gets saved and we have no gallery.

Unless proven that other backends don't work, I am +1 for warning rather
than error.

I thought of the warning as a better alternative than raising the error, because I have no idea of the users use cases. I already updated the PR to give the error some hours ago, going back is not a major trouble. The question I now repeat is why use other backend if Agg outputs to all other formats? Well Cairo does too, but that one is more complicated to install as a dependency. So it a user needs that one he for sure knows why.

I believe the main point of this PR and issue #153 is to tell the user that he is unintentionally loading pylab or pyplot before Sphinx-Gallery can set the backend to AGG. So that he can change his configuration.

@anntzer any comment on this?

@anntzer

This comment has been minimized.

Contributor

anntzer commented Oct 26, 2016

Clearly, interactive backends do not work (because they pop up windows); I think just erroring out clearly with them should be enough.
Whether you want to bother supporting other noninteractive backends is up to you...

try:
# make sure that the Agg backend is set before importing any
# matplotlib
import matplotlib
# Check if we've already set up a backend
current_backend = matplotlib.rcParams['backend']
if 'matplotlib.backends' in sys.modules and current_backend.lower() != 'agg':

This comment has been minimized.

@lesteve

lesteve Oct 26, 2016

Contributor

I think this is simpler:

matplotlib.use('Agg')

if matplotlib.get_backend() != 'Agg':
    raise ValueError(...)
and write them to files. You are currently using the {} backend.
Sphinx-Gallery will terminate the build now, because changing backends
is not well supported by matplotlib. We advise you to review your
Sphinx setup to use the Agg backend.\n\n"""

This comment has been minimized.

@lesteve

lesteve Oct 26, 2016

Contributor

The recommendation should be "Move the sphinx_gallery imports at the top of your conf.py file" rather than "review your configuration", Maybe a more nuanced thing like "sphinx_gallery imports should be before any matplotlib-dependent import. Moving the sphinx_gallery imports at the top of your conf.py file should fix this issue" or something better phrased.

@Titan-C Titan-C force-pushed the Titan-C:matploback branch from 46b1085 to 513108e Nov 7, 2016

@Titan-C Titan-C force-pushed the Titan-C:matploback branch from 513108e to 6ba07d0 Nov 7, 2016

lesteve added some commits Nov 8, 2016

Uniform naming for agg backend
Moving mpl_backend_msg only where it is needed.
@lesteve

This comment has been minimized.

Contributor

lesteve commented Nov 8, 2016

@Titan-C I pushed some minor changes into your branch (github allows maintainers to do that). I'll wait for the CI to come back green and merge this one.

@lesteve

This comment has been minimized.

Contributor

lesteve commented Nov 8, 2016

OK merging this one, thanks @Titan-C.

@lesteve lesteve merged commit a0d9b43 into sphinx-gallery:master Nov 8, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Titan-C Titan-C deleted the Titan-C:matploback branch Dec 17, 2016

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