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.

Sign up for GitHub

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

Jump to bottom

backend_cairo.py -- added print_surface method #1254

Closed
wants to merge 1 commit into from
Closed

backend_cairo.py -- added print_surface method #1254

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/matplotlib/backends/backend_cairo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

dpi = 72
Copy link
Member

Choose a reason for hiding this comment

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

Is this hard-coding intentional? Is there a reason is doesn't take the default dpi from rcParams?

Edit: Should it take the default from rcParams?

Copy link
Author

Choose a reason for hiding this comment

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

The only reason that this is hard coded is because I aped the _save method
which also hard coded the dpi. In my opinion, dpi should be either a named
parameter or read from rcParams.

As far as naming schemes goes, I'm not wedded to anything. I think
print_surface kinda fits the current scheme: print_svg = print to a svg
file, print_surface = print to a cairo surface.

I also have a very basic example of using this method to mix matplotlib and
mapnik in the same document. I'm using a shapefile of California counties
as the example, so the whole package of script, data, and output weighs in
at a little over 7MB. Should I add it to the example directory and issue a
pull request on it?

--Christopher

On Thu, Sep 20, 2012 at 8:50 AM, Damon McDougall
notifications@github.comwrote:

In lib/matplotlib/backends/backend_cairo.py:

@@ -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):
  •    dpi = 72

Is this hard-coding intentional? Is there a reason is doesn't take the
default dpi from rcParams?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1254/files#r1652190.

Copy link
Member

Choose a reason for hiding this comment

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

@chdemars, no, please do not put all that in examples.

Copy link
Member

Choose a reason for hiding this comment

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

@chdemars : Do you keep a blog? I think writing an article about matplotlib & mapnik would be a massive hit. If not, you could always create a small git repo with the contents, maybe even use github's free hosting...
If you want to discuss these options with me, feel free to email me personally/on the mailinglist.

Copy link
Member

Choose a reason for hiding this comment

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

@pelson Yes, yes, yes. Yes a thousand times. The community could learn a lot from specific use-cases such as those of @chdemars. And this is a much more valuable asset than adding 7MB of data to the repository.

self.figure.dpi = dpi
w_in, h_in = self.figure.get_size_inches()
width_pt, height_pt = w_in * dpi, h_in * dpi
renderer = RendererCairo(self.figure.dpi)
renderer.set_width_height(width_pt, height_pt)
renderer.set_ctx_from_surface(fobj)
self.figure.draw(renderer)

def _save (self, fo, format, **kwargs):
# save PDF/PS/SVG
Expand Down