High quality inline SVG figures for notebook. #1881

Closed
wants to merge 8 commits into
from

Projects

None yet

7 participants

@mcelrath
mcelrath commented Jun 7, 2012

Per @ellisonbg's request, I'm submitting this pull request as a place to work on and discuss SVG figures in the notebook. See also #1832.

The basic things I've written so far are:

  1. Increasing figsize to prevent pixelation of svg's.
  2. Some CSS to make svg's fill the browser width. (See #1832 for work on resizable figures)
  3. Modfiy IPython.core.pylabtools.print_figure() to make figures transparent for web use, following matplotlib.savefig().
@ellisonbg

I have code that sets the facecolor to specific colors. I don't think we can set it to none in general. We will probably have to make this configurable.

Member

But this is my point: I have code that I run in the notebook where I create Figures with a black background. These things have to work or minimally be configurable. I don't disagree that for most situations transparent figures would be nicer though.

Aaaa gotcha. I agree this has to be configurable. This is just a first stab, that's why I didn't make a pull request for it, it undoubtedly needs more work.

@fperez
Member
fperez commented Jun 8, 2012

I just want to record here what I said on the list about this topic, I hadn't seen the PR...

On Thu, Jun 7, 2012 at 2:50 PM, Bob McElrath bob+ipython@mcelrath.org wrote:

It turns out to get decent quality SVG figures, you have to set the
figure.figsize fairly high.

Unfortunately it's much more complicated than that. I experimented a
LOT when I first wrote the inline backend, hoping to make SVG the
default. And unfortunately, while most simple plots do indeed look
much nicer as SVG, in production work I kept running into corner cases
where the SVG generated by MPL was just not quite right (missing
elements, clipping lines wrong, poor transparency handling, etc.). Of
course, if I was a good citizen I'd have taken the time to record
those as test cases so we could fix them in MPL, but I just didn't
have the bandwidth to to both ipython and mpl work (this is when we
were just building the kernel system and the qtconsole, so our task
list was monstrous).

I would love to be able to switch to SVG by default, but until we
can trust that the SVG that comes out of MPL is consistently as solid
as the pngs, or have the time to spend fixing those issues, I think
it's best to keep the png defaults.

The problem with fixing this is that I've never been able to see the
issues in easy to reproduce examples. They tend to only happen with
very complex figures that I only can generate pulling full-blown
research datasets out, so they make for poor test cases.

I'll make it a priority from now on to try to at least record these
cases in an isolated place, so that hopefully we can dig into them in
MPL. Because I know it's terrible behavior to put down mpl's
rendering without providing a test case we can use to improve the
situation.

@mcelrath
mcelrath commented Jun 8, 2012

I suspected everyone wanted SVG, but as soon as I turned it on, it was obvious why you didn't make it the default. I think it's clear that the matplotlib svg backend has not received as much love as the others.

Anyway I'll keep this branch here, and I for one will be using it with full-blown research datasets. I'll try to document any badness I encounter.

BTW, @fperez did you ever try using the Cairo backend, and asking it to output svg instead of using the mpl-svg backend? I suspect that will squash most of the cases you're talking about.

@Sharpie
Sharpie commented Jun 8, 2012

Seems like Cairo may be harder to count on for Windows users.

@fperez
Member
fperez commented Jun 8, 2012

@mcelrath no, I didn't even know I could get svg out of the cairo backend in matplotlib! Learn something new every day :)

@mcelrath

I switched the backend to Cairo, but unfortunately the Cairo backend has some kind of font problem. Strings (axis labels, title) sometimes come out as though a cat walked across the keyboard, instead of the intended string.

To switch to the Cairo backend: change line 13 of IPython/zmq/pylab/backend_inline.py from

from matplotlib.backends.backend_svg import new_figure_manager

to:

from matplotlib.backends.backend_cairo import new_figure_manager

(That's all it takes, it seems). None of the other backends will generate svg.

To demonstrate the Cairo font problem, it seems to occur always with the second figure, so enter this in a cell:

%pylab inline
plot(rand(100), label="Test 1")
plt.title("this is a title")
plt.legend()
fig = plt.figure()
plt.title("This is a second title")
plot(rand(100), label="Test 2")
plt.legend()

It was mentioned somewhere that the svg backend has some bugs as well. I've seen this too, for me I often get histograms where the bars extend below the horizontal axis. I don't have a reliable recipe to reproduce it, but maybe that suggestion will be useful to someone.

@fperez
Member
fperez commented Jun 19, 2012

Mmh, if this last one is repeatable, it's definitely worth reporting upstream to mpl. @ivanov, you're our dual-project guru, have you guys seen this before in mpl?

@fperez
Member
fperez commented Jun 25, 2012

Given the recent detailed post on SVG issues, any changes on this front are obviously material for discussion in the 0.14 timeframe, tagging as such.

@ellisonbg
Member

Pinging the status of this PR. Needs to be rebased. @mcelrath what do you think needs to be done on this one?

mcelrath Merge branch 'master' into svg_figures
Conflicts:
	IPython/core/pylabtools.py
	IPython/zmq/pylab/backend_inline.py
be4f491
@mcelrath

It has been rebased.

Only conflict was some code I added to make svg's transparent (makes it easier
to identify the selected cell).

Brian E. Granger [notifications@github.com] wrote:

Pinging the status of this PR. Needs to be rebased. @mcelrath what do you think
needs to be done on this one?


Reply to this email directly or view it on GitHub.

*

Cheers, Bob McElrath

"The individual has always had to struggle to keep from being overwhelmed by
the tribe. If you try it, you will be lonely often, and sometimes frightened.
But no price is too high to pay for the privilege of owning yourself."
-- Friedrich Nietzsche

@travisbot

This pull request passes (merged be4f491 into 144f08a).

@minrk minrk commented on an outdated diff Aug 23, 2012
IPython/core/pylabtools.py
@@ -97,10 +97,22 @@ def print_figure(fig, fmt='png'):
fc = fig.get_facecolor()
ec = fig.get_edgecolor()
- bytes_io = BytesIO()
- fig.canvas.print_figure(bytes_io, format=fmt, bbox_inches='tight',
- facecolor=fc, edgecolor=ec)
- data = bytes_io.getvalue()
+ original_axes_colors = []
+ for ax in fig.axes:
+ patch = ax.patch
+ original_axes_colors.append((patch.get_facecolor(),
+ patch.get_edgecolor()))
+ patch.set_facecolor(fc)
+ patch.set_edgecolor(fc)
@minrk
minrk Aug 23, 2012 IPython member

typo: s/fc/ec/

@minrk
Member
minrk commented Aug 23, 2012

You merged upstream master into this branch a few times. Please avoid doing that by using git rebase.

@minrk minrk commented on an outdated diff Aug 23, 2012
IPython/zmq/pylab/backend_inline.py
@@ -56,7 +56,7 @@ def _config_changed(self, name, old, new):
inline backend."""
)
- figure_format = CaselessStrEnum(['svg', 'png'], default_value='png', config=True,
+ figure_format = CaselessStrEnum(['svg', 'png'], default_value='svg', config=True,
@minrk
minrk Aug 23, 2012 IPython member

Let's not change the default value. This PR makes the SVGs sharper, but does not actually fix the many problems with SVG figures that prompted us to change to PNG from SVG in the first place.

@minrk minrk and 1 other commented on an outdated diff Aug 23, 2012
IPython/core/pylabtools.py
@@ -88,7 +88,7 @@ def figsize(sizex, sizey):
matplotlib.rcParams['figure.figsize'] = [sizex, sizey]
-def print_figure(fig, fmt='png'):
+def print_figure(fig, fmt='svg'):
@minrk
minrk Aug 23, 2012 IPython member

as below, no reason to change the default value here.

@mcelrath
mcelrath Aug 23, 2012

I agree, but how can I inform passers-by of what this branch is supposed to
be/do, and that they should run

%pylab inline
%config InlineBackend.figure_format='svg'

in their notebook?

Min RK [notifications@github.com] wrote:

In IPython/zmq/pylab/backend_inline.py:

@@ -56,7 +56,7 @@ def _config_changed(self, name, old, new):
inline backend."""
)

  • figure_format = CaselessStrEnum(['svg', 'png'], default_value='png', config=True,
  • figure_format = CaselessStrEnum(['svg', 'png'], default_value='svg', config=True,

Let's not change the default value. This PR makes the SVGs sharper, but does
not actually fix the many problems with SVG figures that prompted us to change
to PNG from SVG in the first place.


Reply to this email directly or view it on GitHub.

Min RK [notifications@github.com] wrote:

In IPython/core/pylabtools.py:

@@ -88,7 +88,7 @@ def figsize(sizex, sizey):
matplotlib.rcParams['figure.figsize'] = [sizex, sizey]

-def print_figure(fig, fmt='png'):
+def print_figure(fig, fmt='svg'):

as below, no reason to change the default value here.


Reply to this email directly or view it on GitHub.

Cheers, Bob McElrath

"The individual has always had to struggle to keep from being overwhelmed by
the tribe. If you try it, you will be lonely often, and sometimes frightened.
But no price is too high to pay for the privilege of owning yourself."
-- Friedrich Nietzsche

@minrk
minrk Aug 23, 2012 IPython member

how can I inform passers-by of what this branch is supposed to be/do, and that they should run
%pylab inline
%config InlineBackend.figure_format='svg'

That's always been how you enable SVG figures, and SVG figures are nothing new. So it should be clear simply from the fact that this PR relates to SVG figures that this is required, or if you don't think so, then simply mentioning that once (as you just did) should be quite sufficient.

@mcelrath
mcelrath Aug 23, 2012

It took me quite some time to figure that out.

I'll change it, this is a documentation issue. Google "ipython svg" or "ipython
notebook svg" and none of the links I see lead you to do a %config.

Likewise it took me some time to figure out that "%pylab inline" was the right
thing to be doing...

Min RK [notifications@github.com] wrote:

In IPython/core/pylabtools.py:

@@ -88,7 +88,7 @@ def figsize(sizex, sizey):
matplotlib.rcParams['figure.figsize'] = [sizex, sizey]

-def print_figure(fig, fmt='png'):
+def print_figure(fig, fmt='svg'):

how can I inform passers-by of what this branch is supposed to be/do, and
that they should run
%pylab inline
%config InlineBackend.figure_format='svg'

That's always been how you enable SVG figures, and SVG figures are nothing new.
So it should be clear simply from the fact that this PR relates to SVG figures
that this is required, or if you don't think so, then simply mentioning that
once (as you just did) should be quite sufficient.


Reply to this email directly or view it on GitHub.

*

Cheers, Bob McElrath

"The individual has always had to struggle to keep from being overwhelmed by
the tribe. If you try it, you will be lonely often, and sometimes frightened.
But no price is too high to pay for the privilege of owning yourself."
-- Friedrich Nietzsche

@minrk
minrk Aug 23, 2012 IPython member

I'll change it, this is a documentation issue.

Indeed, our documentation has failed pretty spectacularly at keeping up with changes. Right now, honestly the best docs for how to configure IPython are the generated files you get from:

ipython profile create

Which is pretty sad.

Likewise it took me some time to figure out that "%pylab inline" was the right thing to be doing...

This is directly a result of the non-googlability of notebooks, which we need to address. There shouldn't be a single IPython example notebook with plots that doesn't have this line, so looking at any example notebooks should have revealed it.

@travisbot

This pull request passes (merged 0e2b568 into 144f08a).

@travisbot

This pull request passes (merged 5552809 into 144f08a).

@fperez
Member
fperez commented Sep 6, 2012

@mcelrath, let us know if you need a hand with rebasing this so there are no merges from master. A PR should never have any merges from master into it, as that makes the DAG and the PR harder to analyze, since new code from master shows up in the PR itself.

@Carreau
Member
Carreau commented Sep 30, 2012

Pinging the status of this PR.
There is still a merge status in the middle. So it cannot be merged in master.
Do @mcelrath want to rebase it correctly ? Or should we do it.

When this is done, Is there any things to add, or could it be merged into master ?

@ellisonbg
Member

@fperez I would like to merge this one but the rebase is beyond my git ninja skills. Do you (or someone else) want to have a go at it? Or should be just merge it as is?

@ellisonbg ellisonbg commented on the diff Dec 4, 2012
IPython/zmq/pylab/backend_inline.py
@@ -40,12 +40,12 @@ def _config_changed(self, name, old, new):
# The typical default figure size is too large for inline use,
# so we shrink the figure size to 6x4, and tweak fonts to
# make that fit.
- rc = Dict({'figure.figsize': (6.0,4.0),
+ rc = Dict({'figure.figsize': (32.0,20.0),
@ellisonbg
ellisonbg Dec 4, 2012 IPython member

Won' this change the figure size of all figure types, not just svg?

@ellisonbg
ellisonbg Dec 4, 2012 IPython member

Also, the proportions of the figures was not preserved. Is there a reason for this?

@ellisonbg
ellisonbg Dec 4, 2012 IPython member

I have confirmed that this resizing of figures completely messes up non-svg figures. We simply can't do this.

@mcelrath
mcelrath Dec 4, 2012

These size changes were done because they improve the appearance of SVG figures.
With lower figsize, pixelation and aliasing occurs, which makes SVG's look like
shit. I still do not fully understand why this occurs, but simply increasing
these sizes visually fixes it on screen.

I would be happy to understand why SVG, which is supposed to be a vector format,
has such problems with pixelation and aliasing. This occurs cross-browser, and
even in inkscape, IIRC. I suspect the matplotlib output code, but was unable to
identify what it was putting in the SVG file that caused it.

Brian E. Granger [notifications@github.com] wrote:

In IPython/zmq/pylab/backend_inline.py:

@@ -40,12 +40,12 @@ def _config_changed(self, name, old, new):
# The typical default figure size is too large for inline use,
# so we shrink the figure size to 6x4, and tweak fonts to
# make that fit.

  • rc = Dict({'figure.figsize': (6.0,4.0),
  • rc = Dict({'figure.figsize': (32.0,20.0),

Won' this change the figure size of all figure types, not just svg?

Yes.

Also, the proportions of the figures was not preserved. Is there a reason for
this?

Solely aesthetic on my screen.

Brian E. Granger [notifications@github.com] wrote:

In IPython/zmq/pylab/backend_inline.py:

     # 12pt labels get cutoff on 6x4 logplots, so use 10pt.
  •    'font.size': 10,
    
  •    'font.size': 32,
    

The font size increase is also not proportional. Comments?

Again, chosen by just looking at it and seeing that it looks nice.

Cheers, Bob McElrath

"The individual has always had to struggle to keep from being overwhelmed by
the tribe. If you try it, you will be lonely often, and sometimes frightened.
But no price is too high to pay for the privilege of owning yourself."
-- Friedrich Nietzsche

@ellisonbg
ellisonbg Dec 6, 2012 IPython member

OK I would prefer if we scaled the image proportions and font sizes in the same was.

But, have we asked the mpl devs about the SVG resolution question?

@ellisonbg ellisonbg commented on the diff Dec 4, 2012
IPython/zmq/pylab/backend_inline.py
# 12pt labels get cutoff on 6x4 logplots, so use 10pt.
- 'font.size': 10,
+ 'font.size': 32,
@ellisonbg
ellisonbg Dec 4, 2012 IPython member

The font size increase is also not proportional. Comments?

@ellisonbg
ellisonbg Dec 4, 2012 IPython member

I have also confirmed that the fonts are not being scaled properly.

@ellisonbg ellisonbg commented on the diff Dec 4, 2012
IPython/frontend/html/notebook/static/css/notebook.css
@@ -307,6 +321,13 @@ div.text_cell_render {
overflow-x: auto; /* Changed from auto to remove scrollbar */
}
+/* The selected cell */
@ellisonbg
ellisonbg Dec 4, 2012 IPython member

This turns the selected cell an odd color. I don't recall discussing this. Shouldn't this be removed from the PR?

@mcelrath
mcelrath Dec 4, 2012

Brian E. Granger [notifications@github.com] wrote:

In IPython/frontend/html/notebook/static/css/notebook.css:

@@ -307,6 +321,13 @@ div.text_cell_render {
overflow-x: auto; /* Changed from auto to remove scrollbar */
}

+/* The selected cell */

This turns the selected cell an odd color. I don't recall discussing this.
Shouldn't this be removed from the PR?

I've always had trouble identifying the selected cell (the border change is
insufficient...). Anyway this is unrelated to SVG stuff and shouldn't be in
this PR.

Cheers, Bob McElrath

"The individual has always had to struggle to keep from being overwhelmed by
the tribe. If you try it, you will be lonely often, and sometimes frightened.
But no price is too high to pay for the privilege of owning yourself."
-- Friedrich Nietzsche

@ellisonbg ellisonbg commented on the diff Dec 4, 2012
IPython/core/pylabtools.py
@@ -97,10 +97,22 @@ def print_figure(fig, fmt='png'):
fc = fig.get_facecolor()
ec = fig.get_edgecolor()
- bytes_io = BytesIO()
- fig.canvas.print_figure(bytes_io, format=fmt, bbox_inches='tight',
- facecolor=fc, edgecolor=ec)
- data = bytes_io.getvalue()
+ original_axes_colors = []
@ellisonbg
ellisonbg Dec 4, 2012 IPython member

Can you comment on the purpose of this change?

@mcelrath
mcelrath Dec 4, 2012

Brian E. Granger [notifications@github.com] wrote:

In IPython/core/pylabtools.py:

@@ -97,10 +97,22 @@ def print_figure(fig, fmt='png'):

 fc = fig.get_facecolor()
 ec = fig.get_edgecolor()
  • bytes_io = BytesIO()
  • fig.canvas.print_figure(bytes_io, format=fmt, bbox_inches='tight',
  •                        facecolor=fc, edgecolor=ec)
    
  • data = bytes_io.getvalue()
  • original_axes_colors = []

Can you comment on the purpose of this change?

As described in the PR, I believe this is related to making PNG's and SVG's
transparent.

@ellisonbg
Member

I am going to close this PR because of inactivity. We would love to have this work finished and merged, but we are going to close this PR until work resumes. I have created an issue to track this PR #2647. I have left comments detailing what needs to be addressed. Here are the highlights:

  • Needs to be rebased to remove merge commits with master.
  • We can't change the MPL figure defaults in the inline backend.
  • Color change to selected cell needs to be reverted.

Again, we would love to see this work started, but we like to keep our PR queue with work that is actively progressing. Please feel free to reopen these things are moving forward.

@ellisonbg ellisonbg closed this Dec 4, 2012
@mcelrath
mcelrath commented Dec 4, 2012

Brian E. Granger [notifications@github.com] wrote:

I am going to close this PR because of inactivity. We would love to have this
work finished and merged, but we are going to close this PR until work resumes.
I have created an issue to track this PR #2647. I have left comments detailing
what needs to be addressed. Here are the highlights:

• Needs to be rebased to remove merge commits with master.
• We can't change the MPL figure defaults in the inline backend.

Changing the figure and font sizes is really all that is required to make
SVG's look reasonable on-screen. One needs a mechanism that changes them when
SVG is selected, and changes them back if PNG is selected.

These sizes are a ridiculous pile of imaginary device measurements anyway (pt,
in of a fictitious paper) and don't correspond to pixels (png) or vectors (svg).
I think there is an error in the in->pt->px->vector->screen pipeline for SVG
that could be in matplotlib (though I couldn't find it) or SVG renderers
(browsers) that essentially this PR worked around by just increasing the size.

• Color change to selected cell needs to be reverted.

Again, we would love to see this work started, but we like to keep our PR queue
with work that is actively progressing. Please feel free to reopen these things
are moving forward.

Will do.

@ellisonbg
Member

OK, InlineBackend._figure_format_changed would allow us to change the settings based on the figure format. It should be pretty simple to do that. But it does mean that we would override anything non-default that was set by the user.

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