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

Add choropleth support #40

Merged
merged 20 commits into from
Mar 29, 2018
Merged

Conversation

akacarlyann
Copy link
Collaborator

@akacarlyann akacarlyann commented Feb 3, 2018

Basic implementation of choropleth map-type with ChoroplethViz and choropleth.html; addresses #4

Copy link
Contributor

@ryanbaumann ryanbaumann left a comment

Choose a reason for hiding this comment

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

Looking great @akacarlyann!

Added a few comments on implementation and design:

  • Add property options and mapox gl layers for the outline of the choropleth.
  • Allow a user to specify a vector tile source as the choropleth geometry and use the data-join technique. This allows a user to create a choropleth without requiring the polygon geometry locally in Python.
  • Add tests for Choropleth viz

// Fly to on click
map.on('click', 'polygons', function(e) {
map.flyTo({
center: e.features[0].geometry.coordinates,
Copy link
Contributor

Choose a reason for hiding this comment

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

@akacarlyann on click for choropleth, we'll want to center the map on the to the mouse pointer click location since the polygon could span a very large area.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed with d5a5435.


// Add data layer
map.addLayer({
"id": "polygons",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make the layer name more descriptive- choropleth-fill. Then a label layer could be choropleth-label

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed with d5a5435.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added "chloropleth-label" layer in 3a5b3e6.

style: '{{ styleUrl }}',
center: {{ center }},
zoom: {{ zoom }},
transformRequest: (url, resourceType)=> {
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll want to change this to the latest master branch version that only adds the URL parameter if the request is to a Mapbox API: https://github.com/mapbox/mapboxgl-jupyter/blob/master/mapboxgl/templates/circle.html#L11

Copy link
Collaborator Author

@akacarlyann akacarlyann Feb 20, 2018

Choose a reason for hiding this comment

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

Addressed with 07bf5e5.

map.on('style.load', function() {

// Add data source
map.addSource("data", {
Copy link
Contributor

@ryanbaumann ryanbaumann Feb 3, 2018

Choose a reason for hiding this comment

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

What if the choropleth source is a vector tile? This would be very common when working with Pandas, for example, when you have a tabular set of data that has a geo identifier like postcode that you want to match to a vector tile polygon source with a feature property matching postcode. https://www.mapbox.com/mapbox-gl-js/example/data-join/.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love to figure out a way to make a choropleth viz in mapboxgl use the same python class constructor whether a user has a local geojson file or has a vector tile source to match to.

mapboxgl/viz.py Outdated
data,
color_property=None,
color_stops=None,
color_default='grey',
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add support for outline-color and outline-width properties here to control the style of the border around the choropleth.

I recommend handing these options by creating a second choropleth-line layer, using the line type in Mapbox GL style spec, where the line-color and line-width can be controlled easily.

Copy link
Collaborator Author

@akacarlyann akacarlyann Feb 20, 2018

Choose a reason for hiding this comment

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

Addressed with 07bf5e5.

@akacarlyann
Copy link
Collaborator Author

@ryanbauman Thanks for the feedback! Haven't worked with vector or raster data in the Mapbox or Leaflet environments before so I'll familiarize myself and continue working on this PR. Looks like #21 is also working on raster layers so I don't want to duplicate implementation. Maybe classes for raster or vector layers that can be stacked with the other viz types?

Are you looking for complete replication of Mapbox's example chloropleth style, hover, etc? That would include the HTML stationary popup that calls out the color-property on hover.

@ryanbaumann
Copy link
Contributor

ryanbaumann commented Feb 6, 2018

@akacarlyann

Looks like #21 is also working on raster layers so I don't want to duplicate implementation.

Correct. You're spot on in this PR - lets keep the choropleth viz type in mapboxgl-jupyter to vector data.

Maybe classes for raster or vector layers that can be stacked with the other viz types?

Yes, working on that feature in another issue here #34 (comment)

Are you looking for complete replication of Mapbox's example chloropleth style, hover, etc? That would include the HTML stationary popup that calls out the color-property on hover.

Yes, that's a great reference starting point. Ideally, the default choropleth would have this basic format: https://bl.ocks.org/ryanbaumann/2636bf33c7f1f6c53eca8fd34675f0bb

akacarlyann added 2 commits February 19, 2018 11:31
…Viz; update url transformRequest function in map init to match implementation in master
@akacarlyann
Copy link
Collaborator Author

@ryanbaumann Made some progress today wrt adding chloropleth-line and chloropleth-label layers to ChloroplethViz and its template. Still need to add tests/docs and support for vector tile sources.

@ryanbaumann
Copy link
Contributor

Awesome progress @akacarlyann! I'm happy to help with the vector tile source code if you can hit docs and tests for the existing functionality.

@ryanbaumann
Copy link
Contributor

@akacarlyann if you've already started on Sphynx docs, let me know. We're planning to use a simpler approach to docs as a markdown page in the first release. See the PR at #46

@akacarlyann
Copy link
Collaborator Author

@ryanbaumann I'm happy to add to the markdown development (familiar with that format) and documentation of the code I've contributed. Haven't written up anything in Sphinx yet but I have some notes. Let me know how you me want to integrate the chloropleth docs (fork and PR the current markdown PR?).

@ryanbaumann
Copy link
Contributor

ryanbaumann commented Feb 23, 2018

@akacarlyann yes, let's move on Markdown docs. I'm going to prepare a small release here tomorrow morning Pacific time around 9AM - we'll merge in the Markdown doc PR right now. Then you can build right from master for the docs here.

@ryanbaumann
Copy link
Contributor

Markdown docs are merged into master @akacarlyann

https://github.com/mapbox/mapboxgl-jupyter/tree/master/docs-markdown

@akacarlyann
Copy link
Collaborator Author

Great. I'll see what I can do with that this weekend :)

@akacarlyann akacarlyann force-pushed the chloropleth-support branch 2 times, most recently from ae36792 to 0a2246d Compare February 27, 2018 01:35
…h documentation to docs-markdown/viz.md template
@akacarlyann
Copy link
Collaborator Author

akacarlyann commented Feb 27, 2018

@ryanbaumann I've pushed commits for adding ChloroplethViz to the markdown docs. I'm at the 85% solution for supporting vector tile sources. Just working on getting both vector tile sources or geojson sources to work with a single color_stops argument and color assignment function now.

@ryanbaumann
Copy link
Contributor

Amazing @akacarlyann. Vector tile sources with data joins are going to be super powerful for users. Thanks for pushing on this - let me know when you're ready for a review.

@akacarlyann
Copy link
Collaborator Author

@ryanbaumann There is no polygon data in the tests/ module. Do you have a polygon dataset that fits well with the existing tests? Otherwise I'll build in something like

@pytest.fixture()
def polygon_data():
    with open('tests/polygons.geojson') as fh:
        return json.loads(fh.read())

probably using the US states population density info I've been working with.

@ryanbaumann
Copy link
Contributor

@akacarlyann that's excellent re: using a small local tileset like US States. Natural Earth is a great source of simplified small polygon boundaries for testing - http://www.naturalearthdata.com/downloads/110m-physical-vectors/

@ajfriend
Copy link

This is awesome work! I'll use it immediately.

And, apologies for the random nit from the internet, but as far as naming goes, what's the difference between "chloropleth" and "choropleth" (which I've seen more often)?

Copy link
Contributor

@ryanbaumann ryanbaumann left a comment

Choose a reason for hiding this comment

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

@akacarlyann added small feedback on handling popup creation from joinData


{% block choropleth %}

var joinData = {{ joinData }};
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't want to store the raw data, since we re-map this into an object for the tooltip and array for the style below. Scope this as a function and import the variable in the function scope using let joinData = {{ joinData }}.


{% if joinData %}
var vectorColorStops = [],
popUpKeys = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Store popup keys in an object {} for quick retrieval by key.


{% endblock choropleth %}

{% block choropleth_popup %}
Copy link
Contributor

@ryanbaumann ryanbaumann Mar 18, 2018

Choose a reason for hiding this comment

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

When I run this example code, the mousemove function below is not applied to the map. I believe because the choropleth_popup template in choropleth.html is being executed instead of this block.

I'm sure this could be addressed multiple ways, but I confirmed a quick fix by adding the same block name choropleth_popup to both choropleth.html and vector_choropleth.html with the complete code for the popup specific to that template in each.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I've covered this in 722d747, but I wasn't able to replicate the original problem. Can you confirm if it's working now?

@akacarlyann
Copy link
Collaborator Author

@ryanbaumann I finally got the color logic working over the weekend! I'm cleaning up my last commits now.

…; include helper function for interpolating (mapping) colors when using vector-join technique; update docs and example notebook for vector data support in ChoroplethViz
akacarlyann added 2 commits March 23, 2018 02:56
…yle; import Color as Colour from colour to avoid conflict with chroma package
@ryanbaumann
Copy link
Contributor

Amazing @akacarlyann, thank you! Let me know when you're ready for a review.

@akacarlyann
Copy link
Collaborator Author

@ryanbaumann It's ready for review now :) I took a stab at tests for the new functions I introduced but I need a little help getting those on track. I also noticed when merging master back in to my branch that there is an import from colour that conflicts with mine from chroma-py, so I renamed one of the Color classes. I'm happy to refactor if these serve similar purposes, just let me know!

@ryanbaumann
Copy link
Contributor

Awesome - I'll get you a PR review by Saturday, thanks @akacarlyann

Copy link
Contributor

@ryanbaumann ryanbaumann left a comment

Choose a reason for hiding this comment

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

Awesome @akacarlyann. Made a few suggestions on the implementation of the vector-tile data join version, looks great. I'll add more comments here for tests later today.

RE: Adding comments on tests - your tests are failing just due to the coverage requirement. I'd recommend adding tests for:

  1. generate_vector_color_map in viz.py - https://coveralls.io/builds/16141846/source?filename=mapboxgl/viz.py#L348
  2. color_map in viz.py - https://coveralls.io/builds/16141846/source?filename=mapboxgl/utils.py#L177

Once this merges I'll add an example notebook showing how to create a data-join choropleth from a Pandas dataframe.

@@ -35,7 +15,7 @@
"source": "vector-data",
"source-layer": "{{ vectorLayer }}",
"paint": {
"fill-color": generatePropertyExpression("match", "{{ vectorJoinColorProperty }}", vectorColorStops, "{{ defaultColor }}"),
"fill-color": generatePropertyExpression("match", "{{ vectorJoinColorProperty }}", {{ vectorColorStops }}, "{{ defaultColor }}"),
"fill-opacity": {{ opacity }}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Add layer filter here (see comment on how to create)


{% block choropleth_popup %}

// extract JSON property used for data-driven styling to add to popup
Copy link
Contributor

@ryanbaumann ryanbaumann Mar 24, 2018

Choose a reason for hiding this comment

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

Move this section to create the popup join data before the layer creation in the map.on(style.load) function. We'll want to generate a filter expression here too that limits the vector features visible to ones that match data in the user's data.

Also, I changed the syntax for the color data-join style back to the old mapboxgl property functions in the code below. It's faster than the new expression syntax for this use case and won't be completely deprecated for a while yet, so I switched the example below to use that syntax.

Here's how I'd recommend doing it:

{% block choropleth %}
    
    // extract JSON property used for data-driven styling to add to popup
    {% if joinData %}

        let joinData = {{ joinData }};
        # Create filter for layers from join data
        let layerFilter = ['in', "{{ vectorJoinColorProperty }}"]
        var popUpKeys = {};

        joinData.forEach(function(row, index) {
            popUpKeys[row["{{ dataJoinProperty }}"]] = row["{{ colorProperty }}"];
            layerFilter.push(row["{{ dataJoinProperty }}"])
        });

    {% endif %}

    // Add vector data source
    map.addSource("vector-data", {
        type: "vector",
        url: "{{ vectorUrl }}",
    });

    // Add layer from the vector tile source with data-driven style
    map.addLayer({
        "id": "choropleth-fill",
        "type": "fill",
        "source": "vector-data",
        "source-layer": "{{ vectorLayer }}",
        "paint": {
            "fill-color": {
                "type": "categorical",
                "property": "{{ vectorJoinColorProperty }}", 
                "stops": {{ vectorColorStops }}, 
                "default": "{{ defaultColor }}"
            },
            "fill-opacity": {{ opacity }}
        },
        filter: layerFilter
    }, "{{ belowLayer }}");

    // Add border layer
    map.addLayer({
        "id": "choropleth-line",
        "source": "vector-data",
        "source-layer": "{{ vectorLayer }}",
        "type": "line",
        "layout": {
            "line-join": "round",
            "line-cap": "round"
        },
        "paint": {
            {% if lineDashArray %}
                "line-dasharray": {{ lineDashArray }},
            {% endif %}
            "line-color": "{{ lineColor }}",
            "line-width": {{ lineWidth }},
            "line-opacity": {{ opacity }}
        },
        filter: layerFilter
    }, "{{ belowLayer }}" );

    // Add label layer
    map.addLayer({
        "id": "choropleth-label",
        "source": "vector-data",
        "source-layer": "{{ vectorLayer }}",
        "type": "symbol",
        "layout": {
            {% if labelProperty %}
            "text-field": "{{ labelProperty }}",
            {% endif %}
            "text-size" : generateInterpolateExpression('zoom', [[0,8],[22,16]] ),
            "text-offset": [0,-1]
        },
        "paint": {
            "text-halo-color": "white",
            "text-halo-width": 1
        },
        filter: layerFilter
    }, "{{belowLayer}}" );

{% endblock choropleth %}


{% block choropleth_popup %}
// Show the popup on mouseover
Copy link
Contributor

Choose a reason for hiding this comment

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

Add in the logic to remove the popup here on mouseleave.

    map.on('mouseleave', 'choropleth-fill', function() {
        map.getCanvas().style.cursor = '';
        popup.remove();
    });
    
    // Fly to on click
    map.on('click', 'choropleth-fill', function(e) {
        map.flyTo({
            center: e.lngLat,
            zoom: map.getZoom() + 1
        });
    });

@ryanbaumann
Copy link
Contributor

ryanbaumann commented Mar 25, 2018

Added notes on tests @akacarlyann - just need a bit of additional coverage and you'll be good to go. Adding in a few more tests for the two functions I called out in the comment above should do the trick.

@ryanbaumann ryanbaumann added this to the 0.6.0 milestone Mar 25, 2018
This was referenced Mar 25, 2018
@akacarlyann
Copy link
Collaborator Author

@ryanbaumann Thanks, I'll get those in today :)

@akacarlyann akacarlyann force-pushed the chloropleth-support branch 2 times, most recently from c106407 to d80e99e Compare March 29, 2018 04:00
@akacarlyann
Copy link
Collaborator Author

akacarlyann commented Mar 29, 2018

Alrighty, @ryanbaumann , tests and your requested changes ready for review! PS that zip-code census viz looks sweet!

@ryanbaumann
Copy link
Contributor

@akacarlyann this is so good. Thanks for pushing through a difficult PR, laying the foundation for all sorts of new viz types with the data-join technique packaged up!

@ryanbaumann ryanbaumann reopened this Mar 29, 2018
@ryanbaumann ryanbaumann merged commit 059e39b into mapbox:master Mar 29, 2018
@akacarlyann akacarlyann deleted the chloropleth-support branch December 2, 2019 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants