Skip to content

Make hexbin much faster #859

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

Merged
merged 3 commits into from
May 22, 2012
Merged

Make hexbin much faster #859

merged 3 commits into from
May 22, 2012

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented May 8, 2012

By creating a PolygonCollection containing a single polygon and a number of offsets. This allows the Agg backend to be dealing with far less data, and it allows the vector backends to write the polygon once and "use" it multiple times.

@efiring, @astraw, @ddale all seem to have had a hand in hexbin, so may be interested.

@jkseppan
Copy link
Member

Looks good to me. In the hexbin_extent test, the pdf file size drops to about 4% of the original, the svg file to about 57%.

@mdboom
Copy link
Member Author

mdboom commented May 13, 2012

Yes -- the filesize difference is dramatic. And notably the memory consumption of ghostscript when rendering the PDF file is much lower.

@mdboom
Copy link
Member Author

mdboom commented May 14, 2012

Down the road, we should probably put the smaller files in the repository as the baseline images -- but for now it's probably good to keep the old ones to ensure that the new approach works correctly.

@WeatherGod
Copy link
Member

Can we get a writeup in the CHANGELOG or in api_changes.rst on this. A lot of this is going right over my head and I doubt I can grok this anytime soon.

@mdboom
Copy link
Member Author

mdboom commented May 17, 2012

I generally reserve the CHANGELOG for API changes -- this is merely a behind the scenes change. But how about this?

hexbin plotting now generates files that are much smaller in size since the same hexagon shape is reused for the entire plot.

@WeatherGod
Copy link
Member

I was more referring to all of the features added to backend_bases.py/_backend_agg.* and such. Is it just me, or could they be useful elsewhere as well?

@@ -359,6 +366,24 @@ def get_offsets(self):
else:
return self._uniform_offsets

def set_offset_position(self, offset_position):
"""
The how offsets are applied. If *offset_position* is 'screen'
Copy link
Member

Choose a reason for hiding this comment

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

Docstring glitch.

@efiring
Copy link
Member

efiring commented May 22, 2012

@mdboom, I suggest you fix the docstring and then go ahead and commit. I don't see any point in waiting longer. This is quite an improvement.

mdboom added 3 commits May 22, 2012 12:00
… polygon and a number of offsets. This allows the Agg backend to be dealing with far less data, and it allows the vector backends to write the polygon once and "use" it multiple times.
@mdboom
Copy link
Member Author

mdboom commented May 22, 2012

@WeatherGod: The new setting ("offset_position") is documented in the Collections:get_offset_position/set_offset_position docstrings (improved in the most recent commit), and the new flag to the backend method is documented in RendererBase:draw_path_collection. I've also added a CHANGELOG entry.

Assuming that addresses your concerns, I'll go ahead and merge.

@WeatherGod
Copy link
Member

Yes, I think that made things much clearer for me. Thank you.

mdboom added a commit that referenced this pull request May 22, 2012
@mdboom mdboom merged commit 1f2a750 into matplotlib:master May 22, 2012
@mdboom mdboom deleted the hexbin-simplified branch March 3, 2015 18:43
@anntzer anntzer mentioned this pull request May 22, 2019
6 tasks
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.

4 participants