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

backend_cairo.py -- added print_surface method #1254

Closed
wants to merge 1 commit into from

Conversation

chdemars
Copy link

Added ability to print to a supplied cairo surface. It was the least hackish way I could interface matplotlib and mapnik.

Added ability to print to a supplied cairo surface. It was the least hackish way I could interface matplotlib and mapnik.
@@ -435,6 +435,16 @@ def print_svg(self, fobj, *args, **kwargs):

def print_svgz(self, fobj, *args, **kwargs):
return self._save(fobj, 'svgz', *args, **kwargs)

def print_surface(self, fobj, *args, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

The other methods, as far as I can tell, are named print_[filetype]. Since 'surface' is not a filetype, perhaps this function should be called something else? Perhaps this is really nit-picky, though. I have also just realised it doesn't actually save a file, it merely renders it to a canvas. The other print_[filetype] methods write a physical file to disk. Perhaps render_to_canvas would be more appropriate?

Maybe some of the other devs could chime in here.

Edit: Perhaps also a docstring describing what fobj is and what are vaid args and kwargs. On further inspection, it doesn't look like the args and kwargs get passed anywhere. Is this intentional?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for seeing this late. I think I agree with @dmcdougall here, let's call it "print_to_surface", and the fobj argument name should be surface (or whatever the convention is in cairo), since it is definitely not a file object.

@dmcdougall
Copy link
Member

It looks like this addition renders the current figure to the canvas. It provides convenience. Therefore, should there be one of these added for all the other backends?

@pwuertz
Copy link
Contributor

pwuertz commented Sep 20, 2012

This is a convenience function for creating a Renderer for an existing canvas (or cairo surface in this specific case). Wouldn't it be better to add this stuff to the RendererCairo then? If you are planning to update a GUI, it's probably more efficient to keep the Renderer instead of recreating it every time..

@pelson
Copy link
Member

pelson commented Apr 18, 2013

Hmmm, I think @pwuertz's idea is a good one here.

@chdemars - did you ever manage to write this up?

@WeatherGod
Copy link
Member

It may just be me because I have never used Cairo, but I still have no clue what it is that this "feature" does. Documentation somewhere would be of great use. At first, I thought this had something to do with mplot3d's plot_surface().

@pelson
Copy link
Member

pelson commented Jan 9, 2014

@chdemars - I'm very interested in this functionality - if you can, please do take another look at the suggestion from @pwuertz.

In the meantime, I'm going to close the PR, as it is not quite in a state where we can merge it.

Cheers!

@pelson pelson closed this Jan 9, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants