{{ message }}

# Add support for Polygons with holes#3092

Merged
merged 37 commits into from Oct 23, 2018
Merged

# Add support for Polygons with holes#3092

merged 37 commits into from Oct 23, 2018

## Conversation

### philippjfr commented Oct 19, 2018 • edited

 In conjunction with the corresponding geoviews PR this represents a full protocol for round-tripping data between the dictionary format in holoviews and shapely geometries. Therefore this formally defines the conventions of our format relative to GEOS geometry definitions. A `MultiInterface` can hold a `list` of standard columnar data structures, the columnar data structures may be, numpy arrays, dictionaries of columns, (pandas/dask) dataframes and dictionaries of a geometry and other data. Each item in a `MultiInterface` `list` represents an individual geometry, where a geometry can be a LinearString, LinearRing, Polygon, MultiLineString or MultiPolygon. If an item in the list represents a MultiLineString or MultiPolygon then each polygon/line string should be separated by nans. In order to store holes for MultiPolygons the holes are deeply nested to unambiguously assign each hole to each polygon: 1st. The first level of nesting corresponds to the list of geometries 2nd. The second level corresponds to each Polygon in a MultiPolygon 3rd. The third level of nesting allows for multiple holes per Polygon To demonstrate this let's take a few examples of polygon geometries with holes. In the simplest case we have a Polygon of a few coordinates with holes defined as a list of list of arrays: ```coords = [(1, 2), (2, 0), (3, 7)] holes = [[[(1.5, 2), (2, 3), (1.6, 1.6)], [(2.1, 4.5), (2.5, 5), (2.3, 3.5)]]] hv.Polygons([{('x', 'y'): coords, 'holes': holes}])``` In the case of a `MultiPolygon` which contains the Polygon from above it becomes clearer. The coords now contain a nan separator and there are two separate lists of holes, one the same as above the second empty: ```coords = [(1, 2), (2, 0), (3, 7), (np.nan, np.nan), (6, 7), (7, 5), (3, 2)] holes = [[[(1.5, 2), (2, 3), (1.6, 1.6)], [(2.1, 4.5), (2.5, 5), (2.3, 3.5)]], []] hv.Polygons([{('x', 'y'): coords, 'holes': holes}])``` To further illustrate this, we can split the `MultiPolygon` into two separate Polygons like this: ``````coords = [(1, 2), (2, 0), (3, 7)] holes = [[[(1.5, 2), (2, 3), (1.6, 1.6)], [(2.1, 4.5), (2.5, 5), (2.3, 3.5)]]] coords2 = [(6, 7), (7, 5), (3, 2)] hv.Polygons([{('x', 'y'): coords, 'holes': holes, 'value': 1}, {('x', 'y'): coords2, 'value': 2}], vdims='value') `````` producing almost the same plot but allowing us to set two distinct values for each polygon, when in the MultiPolygon case they would have had to share the same value. This scheme can also be used to faithfully round-trip matplotlib geometries (e.g. Patches) to this format, which means we can finally handle the `contours` operation correctly. Closes #2017 Rewrite contours operation to convert matplotlib Patches correctly Added Geometry data user guide Add unit tests
added 3 commits Oct 19, 2018
``` Do not validate unreferenced dict columns ```
``` 2e7c6e1 ```
``` Add API to return holes from MultiInterface ```
``` 8b94889 ```
``` Unpack holes in bokeh ContourPlot ```
``` 5a324f7 ```

### jsignell commented Oct 19, 2018

 Are you losing the mapping from polygon (section of multipolygon) to hole in that transformation?

### jsignell commented Oct 19, 2018

 nevermind I misread. I think that seems like a reasonable representation. It might be worth looking at geopandas though and trying to make that work well

### philippjfr commented Oct 19, 2018

 nevermind I misread. I think that seems like a reasonable representation. It might be worth looking at geopandas though and trying to make that work well Geopandas support will follow in geoviews, but there we don't have to worry about the representation since geopandas already represents holes.

### jsignell commented Oct 19, 2018

 there we don't have to worry about the representation since geopandas already represents holes. Oh I see, so we won't transform it to this style?

### philippjfr commented Oct 19, 2018

 Oh I see, so we won't transform it to this style? No, it should generally stay in its native format, i.e. inside shapely objects.

added 8 commits Oct 21, 2018
``` Simplified DictInterface constructor ```
``` 9fd0e6a ```
``` Handle nans when detecting scalar dimensions ```
``` 9e0a455 ```
``` Fixed nan warning in range calculation ```
``` 9d26fac ```
``` Allow ContoursPlot to declare plot_method explicitly ```
``` 53bf65e ```
``` Always compute Contours from expanded coordinates ```
``` 978bb9c ```
``` Always convert holes to arrays ```
``` 67466ba ```
``` Contours operation handles holes and fixes levels ```
``` a51a795 ```
``` ContourPlot displays colorbar ```
``` 6fc24f6 ```
mentioned this pull request Oct 21, 2018

### philippjfr commented Oct 21, 2018

 Slight correction now that I've started on the geoviews representation, in geoviews paths and polygons are converted to lists of dictionaries containing shapely geometries, which simplifies projections massively.

added 8 commits Oct 22, 2018
``` Handle MultiPolygons correctly ```
``` 03f4d45 ```
``` Simplify bokeh PathPlot ```
``` df854c1 ```
``` Convert polygons to matplotlib paths ```
``` 49d87aa ```
``` Optimization for dictionary range ```
``` 0e4c9d5 ```
``` Fixed issue handling empty contourf holes ```
``` 7b46eb2 ```
``` Overhauled Path/Contours/Polygons docstrings ```
``` 2828290 ```
``` Fixed issue with multi-geometries declaring no holes ```
``` 7fd1c80 ```
``` Added Geometry_Data user guide ```
``` 8155fc1 ```
added 3 commits Oct 22, 2018
``` Fixed contours operation ```
``` f4379fb ```
``` Added unit tests for hole handling ```
``` 90226eb ```
``` Ignore holes if bokeh version <1.0 ```
``` 95eaee1 ```
reviewed
 for i, sd in enumerate(d): unpacked.append((sd, vals[:, i])) unpacked = []

#### philippjfr Oct 22, 2018 Author Member

Note that I simply unindented this section when I thought I'd have to complicate it further, there are no changes to `DictInterface.init` that need reviewing.

reviewed
 unique = set(values) else: unique = np.unique(values) if (~util.isfinite(unique)).all():

#### philippjfr Oct 22, 2018 Author Member

A column of NaNs was not being detected as being scalar.

reviewed

#### jlstevens Oct 22, 2018 Contributor

Remember to clear out this metadata before this PR can be merged...

#### philippjfr Oct 22, 2018 Author Member

Will do.

reviewed
 "\n", "The ``Path`` element represents a collection of path geometries with associated values. Each path geometry may be split into sub-geometries on NaN-values and may be associated with scalar values or array values varying along its length. In analogy to GEOS geometry types a Path is a collection of LineString and MultiLineString geometries with associated values.\n", "\n", "While many different formats are accepted in theory, natively HoloViews provides the ``MultiInterface`` which allows representing paths as lists of regular columnar data objects including arrays, dataframes and dictionaries of column arrays and scalars. A simple path geometry may therefore be drawn using:"

#### jlstevens Oct 22, 2018 Contributor

Are interfaces discussed elsewhere in the user guide? If not, I wouldn't mention `MultiInterface` explicitly, if so, I might want to point to where interfaces are discussed.

#### philippjfr Oct 22, 2018 Author Member

Okay removing this.

reviewed
 "cell_type": "markdown", "metadata": {}, "source": [ "If a polygon has no holes at all the 'holes' key may be ommitted entirely:"

#### jlstevens Oct 22, 2018 Contributor

Maybe worth doing this first (no holes) before introducing holes? I.e a reminded of the `Polygon` constructor before holes were supported...

Sure.

#### philippjfr Oct 22, 2018 Author Member

Actually I think saying that the format is exactly the same as the paths one is enough, otherwise it gets a bit repetitive.

#### jlstevens Oct 22, 2018 Contributor

Sure.

reviewed
 " {'x': [4, 6, 6], 'y': [0, 2, 1], 'value': 1}\n", "], vdims='value')\n", "\n", "polys = poly.split()\n",

#### jlstevens Oct 22, 2018 Contributor

Don't think it is worth introducing the `polys` handle here (just inline it to the `Layout`). Looks to me like the handle that is reused is just `poly`.

#### philippjfr Oct 22, 2018 Author Member

Sounds good.

reviewed
 @@ -185,3 +186,33 @@ def get_raster_array(image): else: data = np.flipud(data) return data def ring_coding(array):

#### jlstevens Oct 22, 2018 Contributor

Is this called `ring` because it is closed (or at least looks closed)? If so the docstring should mention this, if not, I'm not sure what 'ring' is referring to. If I remember right, matplotlib has explicit codes for declaring genuinely closed paths that I assume you've considered using...

#### philippjfr Oct 22, 2018 Author Member

It generates the path codes for each of the exterior and interior paths (which are always rings in a polygon). Using CLOSEPOLY path codes is not necessary since PathPatch will automatically close any patches.

reviewed
 key = Polygons._hole_key if key in dataset.data: return [[[np.asarray(h) for h in hs] for hs in dataset.data[key]]] else:

#### jlstevens Oct 22, 2018 Contributor

Why can't you use the super definition of `holes` if holes aren't defined/supported?

#### jlstevens Oct 22, 2018 Contributor

Unless the default implementation works for all interfaces except this one specifically?

#### philippjfr Oct 22, 2018 Author Member

Probably can do that.

### jlstevens commented Oct 22, 2018

 I've made a few comments but overall this PR looks good. This feature has been a long time coming!

added 5 commits Oct 22, 2018
``` Refactoring of hole handling ```
``` 8f5ee4e ```
``` Added unit tests for bokeh plots of Polygons with holes ```
``` 43b26d7 ```
``` Various fixes for hole handling ```
``` 4cc6ae1 ```
``` Added unit test for matplotlib hole plotting ```
``` fa49912 ```
``` Addressed documentation comments ```
``` f77829f ```
added this to the v1.11.0 milestone Oct 22, 2018
added 6 commits Oct 22, 2018
``` Small contours operation fix ```
``` c4ebc25 ```
``` Fixed flakes ```
``` a29a903 ```
``` Updated Bivariate element test ```
``` 18d960b ```
``` Cast contour levels to array ```
``` 026d700 ```
``` Fixed contours levels integer kwarg ```
``` 0147b3e ```
``` Updated contours tests ```
``` ff319a6 ```

### jlstevens commented Oct 23, 2018

 The tests have now passed (finally!). All looks good to me.

merged commit `cfb4b62` into master Oct 23, 2018
4 checks passed
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.3%) to 88.889%
Details
s3-reference-data-cache Test data is cached.
Details
mentioned this pull request Oct 23, 2018