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 support to show polygons #15

Closed
ustroetz opened this issue Oct 17, 2017 · 7 comments
Closed

Add support to show polygons #15

ustroetz opened this issue Oct 17, 2017 · 7 comments
Assignees
Milestone

Comments

@ustroetz
Copy link
Contributor

ustroetz commented Oct 17, 2017

Hi there,

I would like to add support to show polygons. Before I start with an implementation, I would like to discuss with you the best approach.

I was thinking about the following steps:

  1. Create a new template polygon.html.
  2. Create in viz.py a new Class PolygonViz.
  3. Create in viz.py a new base Class Viz. PolygonViz, GraduatedCircleViz, and CircleViz inherit from this Class. Below is the rough outline of the Viz Class.
  4. Create one polygon example jupyter notebook that proves the implementation.

I am happy for any feedback about the above. Since I am new to the project, I am also open for a complete different approach to show polygons.

Looking forward to work on this 😊


class Viz(object):
    """Base class for all visualisations"""

    def __init__(self,
                 access_token=None,
                 center=(0, 0),
                 div_id='map',
                 height='500px',
                 style_url="mapbox://styles/mapbox/light-v9?optimize=true",
                 width='100%',
                 zoom=0):

        if access_token is None:
            access_token = os.environ.get('MAPBOX_ACCESS_TOKEN', '')
        if not access_token.startswith('pk'):
            raise TokenError('Mapbox access token must be public (pk)')
        self.access_token = access_token

        self.div_id = div_id
        self.width = width
        self.height = height
        self.data = data
        self.style_url = style_url
        self.center = center
        self.zoom = zoom

    def as_iframe(self, html_data):
        """Build the HTML representation for the mapviz."""
        ...
       
    def show(self, **kwargs):
        """Load the HTML iframe and display the  the iframe in the current jupyter notebook view"""
        ...
@ryanbaumann
Copy link
Contributor

ryanbaumann commented Nov 10, 2017

@ustroetz sorry for the delay here - yes, we'd love to see a PR including line and polygon visualization support! I'd recommend using the nomenclature for your line and polygon visualizations as

a) line => line layer type
b) fill (or possibly choropleth) => fill layer type

You are exactly right in your approach. For an example of what adding a new Viz class looks like in total, check out the Heatmap PR - #16

@ryanbaumann
Copy link
Contributor

@ustroetz are you interested in submitting a PR for this feature?

@ustroetz
Copy link
Contributor Author

Yes, I actually have a branch sitting here that I want to finish up :)

I'll try to open a PR by the end of the week 🤞

@ryanbaumann
Copy link
Contributor

There's an open PR starting this work at #40

@ustroetz
Copy link
Contributor Author

ustroetz commented Feb 4, 2018

Glad to see that someone picked up the work! Sorry that I was a bit slow on this.
Awesome to see that there is now so much traction on the project 👏

@ryanbaumann
Copy link
Contributor

@akacarlyann did you get a chance to look at adding in the next step for this issue in your PR #40? Looking at what to include in the next release.

@narlesky
Copy link

@ryanbaumann I have. Not quite done yet but I should have some time opening up in my schedule to work on this later in the week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants