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

Implement __geo_interface__ for relevant classes #621

Merged
merged 2 commits into from
Jun 16, 2020

Conversation

vpipkt
Copy link
Contributor

@vpipkt vpipkt commented Jun 15, 2020

This PR implements the __geo_interface__ spec for GeoJSON, Choropleth, and GeoData. For GeoData we fall back to geopandas implementation on the GeoDataFrame.

Note that as is the convention with GeoDataFrame a {'type': 'FeatureCollection', ...} can be returned. This is not explicitly stated in the spec but the comments lobby for it and ask the geopandas contributors to expand on their approach.

This is my first contribution to the project so I appreciate your attention and patience!

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

It looks good to me :) I wonder if we will need to make something similar for Marker/Polygon classes in the future?

@@ -545,6 +545,16 @@ def _get_data(self):

return self.data

@property
def __geo_interface__(self):
"""
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for making a docstring! We need more of those.

@davidbrochart
Copy link
Member

Awesome @vpipkt !
This is good to merge after fixing the linting issue.

@vpipkt
Copy link
Contributor Author

vpipkt commented Jun 15, 2020

@martinRenou Yes I would say it would be great for Marker and Polygon. Should I add to this PR or make a separate one?

@martinRenou
Copy link
Member

Another PR is fine :) Thanks again!

@martinRenou martinRenou merged commit c0aaf66 into jupyter-widgets:master Jun 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants