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

Fix for bartromgens/geojsoncontour issue#10 #14

Merged
merged 1 commit into from
Sep 24, 2019

Conversation

veloman-yunkan
Copy link
Contributor

In contourf_to_geojson(), every element of contourf.collections is converted into a single GeoJSON MultiPolygon. While doing so every matplotlib path object is converted into a separate GeoJSON Polygon object. Before this fix, the result of contourf_to_geojson() was a single GeoJSON Polygon (wrapped into a MultiPolygon) per GeoJSON Feature made up of all the rings (LineStrings) of all Polygons (i.e. such a polygon could have - in violation of the GeoJSON spec - multiple exterior rings).

The changes in the tests/benchmark_multipolycontourf.geojson file are best displayed by the following command:

git diff --word-diff-regex=.  tests/benchmark_multipolycontourf.geojson

In contourf_to_geojson(), every element of `contourf.collections` is
converted into a single GeoJSON MultiPolygon. While doing so every
matplotlib path object is converted into a separate GeoJSON Polygon
object. Before this fix, the result of `contourf_to_geojson()` was
a single GeoJSON Polygon (wrapped into a MultiPolygon) per GeoJSON Feature
made up of all the rings (LineStrings) of all Polygons (i.e. such a polygon
could have - in violation of the GeoJSON spec - multiple exterior rings).

The changes in the tests/benchmark_multipolycontourf.geojson file are
best displayed by the following command:

    git diff --word-diff-regex=.  tests/benchmark_multipolycontourf.geojson
@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling de543d7 on veloman-yunkan:pr1 into 7cb1e2e on bartromgens:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling de543d7 on veloman-yunkan:pr1 into 7cb1e2e on bartromgens:master.

@veloman-yunkan veloman-yunkan mentioned this pull request Sep 21, 2019
Copy link
Owner

@bartromgens bartromgens left a comment

Choose a reason for hiding this comment

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

Great! Looks good. Thanks.

Both multiple polygons per level, and polygon with hole render as expected by http://geojson.io and http://geojson.tools/ now.

holes
multipolygon

@bartromgens bartromgens merged commit 73d8a84 into bartromgens:master Sep 24, 2019
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