[WIP] MultiPolygons with holes #8340

jsignell · 2018-10-17T20:04:37Z

Adding a new MultiPolygons glyph as outlined here https://github.com/bokeh/bokeh/wiki/Polygons-Patches-with-Holes#new-glyph.

issues: fixes Patches with Holes #2321
tests added / passed
release document entry (if new feature or API change)

Example:

<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Complete Example</title>
<link rel="stylesheet" href="./bokehjs/build/css/bokeh.css" type="text/css" />
<script type="text/javascript" src="./bokehjs/build/js/bokeh.js"></script>
<script type="text/javascript" src="./bokehjs/build/js/bokeh-api.js"></script>
<!-- The order of CSS and JS imports above is important. -->
</head>

<body>
<div>
<script type="text/javascript">

var source = new Bokeh.ColumnDataSource({
    data: {
        //  i is one multipolygon
        //  j is one polygon
        //  k is either an exterior ring (0) or interior ring/hole (1:n)
        xs: [
            [ [ [1, 1, 2, 2]                             ]                    ],
            [ [ [1, 1, 3],    [1.5, 1.5, 2]              ]                    ],
            [ [ [2, 2, 4, 4], [2.5, 2.5, 3], [3.5, 3, 3] ], [ [3.5, 3.5, 4] ] ]
        ],
        ys:
            [
                [                       // multipolygon 0
                    [                   // polygon 0
                        [4, 3, 3, 4]    // exterior
                    ],
                ],
                [                       // multipolygon 1
                    [                   // polygon 0
                        [1, 3, 1],      // exterior
                        [1.5, 2, 1.5]   // hole 0
                    ],
                ],
                [                       // multipolygon 2
                    [                   // polygon 0
                        [2, 4, 4, 2],   // exterior
                        [3, 3.5, 3.5],  // hole 0
                        [2.5, 2.5, 3]   // hole 1
                    ],
                    [                   // polygon 1
                        [1, 1.5, 1.5]   // exterior
                    ]
            ]
        ]
    }
});

var plot = Bokeh.Plotting.figure({title:'New Glyph - MultiPolygons with Holes', height: 300, width: 300, tools: 'hover'});
var patchData = plot.multi_polygons(
    { field: "xs" },
    { field: "ys" },
    { source: source, fill_color:['red', 'limegreen', 'goldenrod']}
);

Bokeh.Plotting.show(plot, document.currentScript.parentElement);

</script>
</div>
</body>
</html>

bryevdv · 2018-10-17T21:20:21Z

Given that the New Glyph approach does not intersect with any existing codepaths, I am sorely tempted to consider this as a late add for 1.0, but "done" also means tests, a user's guide section, a contour plot example, and geojson example.... I think that's probably more than can be accomplished in a few days.

Also, I am not sure more work isn't really needed before this is truly useful for Geo work. I was trying to add a tile source under a GeoJSONDataSource scatter earlier today and gave up because one used lat/lon and the other web-mercator. We'd need to figure out how to make things work well together without users having to do alot of conversions by hand.

Most importantly, though: this is fantastic to finally be close to having, thanks for putting things together @jsignell

jsignell · 2018-10-17T21:31:52Z

Questions:

~~Where should the center of a multipolygon with holes be? Right now I am ignoring holes when getting centers.~~
~~Why isn't the initial range getting set properly (see glyphs example)?~~
~~Is there a more efficient way to check for containment in polygon but not hole?~~
~~Why can't you set fill_color with a list when initializing the class?~~
~~Should we allow un-nested input in more simple cases? (probably not)~~
Should we raise an error if:
- the hole isn't wound clockwise
- two parts of a multipolygon overlap
- ~~a hole intersects the exterior ring~~

jsignell · 2018-10-17T21:32:37Z

@bryevdv I'd be happy to push on this and try to get it in for 1.0, I'll just probably need some more help :)

jbednar · 2018-10-17T21:38:55Z

Where should the center of a multipolygon with holes be? Right now I am ignoring holes when getting centers.

That sounds correct to me.

Should we raise an error if:
a hole intersects the exterior ring

It seems like intersection per se isn't a problem, e.g. a pattern like this has lots of intersections but still seems valid:

The same hole intersecting more than once could be a problem, depending on how it's done, but I'm not sure how to characterize what would be legal and what shouldn't be.

bryevdv · 2018-10-17T21:43:30Z

Where should the center of a multipolygon with holes be? Right now I am ignoring holes when getting centers.

Let's keep things simple at least for now, the center of the exterior ring

Why isn't the initial range getting set properly (see glyphs example)?

I don't think we are using the spatial index at this point? If you put all MBB of all exterior rings for each MPgon in the index in an _index_data method, then I think that would be sufficient. Tho you might need to call it yourself since you are also overriding map_data. See e.g.

https://github.com/bokeh/bokeh/blob/master/bokehjs/src/lib/models/glyphs/patches.ts#L74-L98

EDIT: looks like index_data is called by set_data so just adding the private _index_data delegate should be sufficient

Why can't you set fill_color with a list when initializing the class?

It's a DataSpec, it can either be a fixed value or a reference to a CDS column

Is there a more efficient way to check for containment in polygon but not hole?

I think just check that it is in the exterior ring but not the holes. If you spatially index the exterior rings then that will narrow down the candidates to brute force this way. More sophisticated approach might spatially index the holes too but thats complex enough that it would need to be justified.

Should we allow un-nested input in more simple cases? (probably not)

Let's say no, for now at least

Should we raise an error if:

I think interior overlap is an error, if that is easily detectable. Tho feel free to console.log any situation that you think might have unexpected or ambiguous results for users.

bryevdv · 2018-10-17T21:44:15Z

I'd be happy to push on this and try to get it in for 1.0, I'll just probably need some more help :)

I'm available, then. Let's try to shoot for EOB Friday, and if not no worries we will have a follow on release soon.

jsignell · 2018-10-18T15:32:20Z

bokehjs/src/lib/core/util/serialization.ts

+function traverse_data(v: any, buffers: [any, any][]): [Arrayable, any] {
+  // v is just a regular array of scalars
+  if (v.length == 0 || !(isObject(v[0]) || isArray(v[0]))) {
+    return [v, v.length]


@bryevdv, is there a reason why we weren't setting shape to v.length in this case where v is an array?

The original code that only handled 1d arrays just didn't require it (checked the length directly) So it simply was not added due to lack of necessity. I am not opposed to adding it to make things consistent, but let's make an issue and do that after 1.0 since I can't say with certainty that nothing ever conditions on the presence or absence of shape to deduce array dimension, and adding it now might beak such paths.

bryevdv · 2018-10-18T23:04:01Z

Merging now to get in rc3, more docs/examples to follow

jbednar requested a review from philippjfr October 17, 2018 21:32

jsignell added the status: WIP label Oct 17, 2018

jsignell added 5 commits October 18, 2018 10:52

Functioning example of multipolygons with holes

68c5b12

Fixing indentation and only drawing each multipolygon once

a98810c

Checks whether mouse is in hole

1166bf5

Getting Python side hooked up

bec5273

TS handling more deeply nested cases

24f97cd

jsignell force-pushed the jsignell/multipolygons branch from 1a573fc to 24f97cd Compare October 18, 2018 14:54

jsignell added 2 commits October 18, 2018 11:07

Updating reference example

777fa21

Adding fill to basic test

eed48fd

jsignell commented Oct 18, 2018

View reviewed changes

jsignell and others added 5 commits October 18, 2018 13:41

Adding some js unit tests for serialization

a918637

Fixing up shape serialization to give exactly same as before

dab442c

Linting

1576dd6

Removing logging

067a091

remove tabs

1261953

bryevdv added status: accepted and removed status: WIP labels Oct 18, 2018

bryevdv approved these changes Oct 18, 2018

View reviewed changes

bryevdv merged commit b995b39 into master Oct 18, 2018

bryevdv deleted the jsignell/multipolygons branch October 18, 2018 23:04

rsignell-usgs mentioned this pull request Oct 19, 2018

Confusion with geopandas dataframe as Polygons input holoviz/geoviews#214

Closed

jderehag mentioned this pull request Feb 22, 2019

Graphs with list values as attributes fail #8679

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] MultiPolygons with holes #8340

[WIP] MultiPolygons with holes #8340

jsignell commented Oct 17, 2018 •

edited by jbednar

bryevdv commented Oct 17, 2018

jsignell commented Oct 17, 2018 •

edited

jsignell commented Oct 17, 2018

jbednar commented Oct 17, 2018

bryevdv commented Oct 17, 2018 •

edited

bryevdv commented Oct 17, 2018

jsignell Oct 18, 2018

bryevdv Oct 18, 2018

bryevdv commented Oct 18, 2018

[WIP] MultiPolygons with holes #8340

[WIP] MultiPolygons with holes #8340

Conversation

jsignell commented Oct 17, 2018 • edited by jbednar

bryevdv commented Oct 17, 2018

jsignell commented Oct 17, 2018 • edited

jsignell commented Oct 17, 2018

jbednar commented Oct 17, 2018

bryevdv commented Oct 17, 2018 • edited

bryevdv commented Oct 17, 2018

jsignell Oct 18, 2018

Choose a reason for hiding this comment

bryevdv Oct 18, 2018

Choose a reason for hiding this comment

bryevdv commented Oct 18, 2018

jsignell commented Oct 17, 2018 •

edited by jbednar

jsignell commented Oct 17, 2018 •

edited

bryevdv commented Oct 17, 2018 •

edited