Open jankatins opened 9 years ago
Sorry for the late reply, I'm kinda swamped with work in the last few weeks.
Is this patch for IPython? Would this mechanism simply replace the existing _repr_png
and _repr_svg
methods?
For what it's worth, others have also reported problems with the SVG rendering, although not from the performance side - it seems like the SVG plots embedded in an IPython notebook do not render the vertex labels properly, so I'm very interested in finding a solution that allows the user to at least configure what format the plots are sent to the IPython backend, and I'm happy to make PNG the default if it solves the performance problems.
No problem.
This could replace the current _repr_svg_()
method on Plot
. The idea would be to check on importing whether igraph is in an IPython kernel and call select_igraph_plot_formats(["png"])
. IPython then calls render_igraph_plot(plot, "png")
for the plot object and returns the png-string.
If you want to have a different format, simple call select_igraph_plot_formats(["svg", "pdf"])
and you get both svg and pdf (in this case the notebook just uses the svg output and does not show the pdf, but it's still there in the ipynb
file; only PDF and you get a download link for the PDF).
If 'enable on import' is too much, this could also become a user action, much like %matplotlib inline
for matplotlib plots.
As a sidenote: if you only want to switch to png, the easiest would be to add this method to Plot
:
def _repr_png_(self):
"""returns a PNG representation of this plot as a string.
This method is used by IPython to display this plot inline.
"""
surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, int(self.bbox.width), int(self.bbox.height))
context = cairo.Context(surface)
# Plot the graph on this context
self.redraw(context)
io = BytesIO()
surface.write_to_png(io)
# Return the raw SVG representation
return io.getvalue()
[EDIT: wrong, it will call all magic methods available]IPython will only call [/]_repr_png_
and not anymore _repr_svg_
AFAIK.
I have tried an implementation of _repr_png_
before (in response to a complaint from a user where _repr_svg_
did not work for him in a particular setup), but the problem was that adding _repr_png_
alone would not prevent IPython from using SVG in the notebook if _repr_svg_
is already available, and I could not find any easy way to make IPython prefer PNG over SVG, so I dropped the idea because I did not want to remove SVG support entirely. It looks like your solution would make it possible to add support for all three formats (SVG, PNG and PDF) while also making it possible to use PNG only as the default (to solve others' problems with the SVG rendering), so I'm definitely positive about the idea.
I'm on holiday until the end of this week so let me revisit this thread early next week. I definitely like the idea, but I'd rather try to find a solution where we use the standard configuration mechanism of python-igraph (see the igraph.config
module) to specify the default format instead of a custom function like select_igraph_plot_formats
. This is probably not a big change compared to your solution, and it would also allow us to specify the default figure size in the configuration as well, rendering the set_igraph_figsize_in_ipython
function unnecessary.
Anyway, thanks for the idea and the code samples - I'll definitely get back to this and implement it somehow in the next release. If you feel like doing so, you can fork the master branch of the repo and merge your code into the forked branch, and then I'll revisit your branch once I'm back from holidays.
I won't have time to work on this until friday next week, so no problem...
Ok, just looked it up: IPython will call all magic methods which are available (so _repr_png_
and _repr_svg_
and whatever else there is) and return the result (so the notebook has all formats available, but only displays one).
One way to work around this implementing something that makes getattr(plot_object, "_repr_svg_", None)
return None
if svg is not the prefered format.
Oh, and this should also work: raise a NotImplementedError
in _repr_svg_
if svg should not be computed. The @catch_format_error
decorator around IPython.core.formatters.BaseFormatter.__call__()
should filter that away...
But I feel that the above way with an explicit formatter for Plot
is a much nicer and less hackish way than these two solutions...
The main obstacle in setting image sizes is that currently the final size is added to the plot during plot(...)
and not during rendering, which resulted in this horrible hack. A better solution should probably add margins to the plot object list and compute the bbox during rendering...
So, would it be acceptable to remove _rep_png_
and use the formatter way? And if the latter: enable it on import or should that be an explicit step?
Yes, removing _repr_png_
is acceptable. It would be great if we could somehow detect during import whether we are running inside IPython and then set up the plotting automatically.
Another question: I would like to remove the hack in the patch by pushing the size calculations from the plot()
function into the methods of the Plot
object. this might result in code duplication but then size will be determined in the actual drawing and not before. IMO this will not change the behaviour in non IPython environments and IMO also not in IPython environments. Is this also acceptable?
Yes, it is also acceptable.
Now that matplotlib is an accepted plotting engine for python-igraph, you can use its PNG (or other) static raster backends to achieve this. I'd be for closing this one and telling people to use mpl. But @ntamas this is just my personal preference.
Let's go back to the beginning of this thread. If I understand correctly, the original motivation for this patch was this:
I've found that the svg rendering becomes a real performance hog when the plotted graph gets big.
The question is: is this still a problem, and if it is, is this solved with the Matplotlib backend? If Matplotlib solves this, then we can safely close this issue. Otherwise we should attempt to find a solution somehow, first by figuring out where the performance bottleneck is. From a cursory look at the original proposal, it still draws the graph through Cairo by the usual means but avoids jumping through a few hoops, so the original performance bottleneck is somewhere in the machinery around the Cairo plotting and not in Cairo itself.
Might be worth mentioning that we are considering switching to matplotlib as default. Mpl deals with backends (e.g. PNG/SVG) quite flexibly and is generally quite well integrated with notebooks and ipython as well, so if we transition we might end up in the same boat as many other libraries in terms of performance: might not support billions of nodes, but it will let the user choose the backend at will.
I've found that the svg rendering becomes a real performance hog when the plotted graph gets big.
I've build the following functions, which are enabled similar to the version which manages the image plotting of matplotlib figures:
Usage:
Would you take such a patch?