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

Fixes for Polygons plotting with holes #3409

Merged
merged 6 commits into from Jan 17, 2019

Conversation

Projects
None yet
1 participant
@philippjfr
Copy link
Contributor

philippjfr commented Jan 17, 2019

The PolygonPlot implementation switched between the MultiPolygons and Patches glyphs depending on whether the polygons have holes or not. The problem with that is that in a dynamic plot or HoloMap the returned data format may therefore change but the glyph is only initialized once, so a MultiPolygons glyph could end up with the data format for the Patches glyph and vice versa. This PR ensures that Polygons are always plotted using the MultiPolygons glyph unless a PolyDraw or PolyEdit stream is attached to the plot, which do not yet support holes. This ensures backwards compatibility while also fixing the bug.

  • Adds unit tests
@@ -284,7 +284,7 @@ def range(self, dim, data_range=True, dimension_range=True):
return (None, None)
elif all(util.isfinite(v) for v in dim.range) and dimension_range:
return dim.range
elif dim in self.dimensions() and data_range and len(self):
elif dim in self.dimensions() and data_range and bool(self):

This comment has been minimized.

@philippjfr

philippjfr Jan 17, 2019

Author Contributor

This is mainly an optimization, computing the length on a Polygons/Path can be very expensive, bool(self) is equivalent but a much quicker way to check whether the element is empty.

@philippjfr philippjfr force-pushed the poly_fixes branch from 284ba4a to 091811e Jan 17, 2019

@philippjfr philippjfr merged commit 9a6a630 into master Jan 17, 2019

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.006%) to 90.109%
Details
s3-reference-data-cache Test data is cached.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.