-
Notifications
You must be signed in to change notification settings - Fork 366
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: contourf nested contours #2225
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,15 +168,20 @@ def path_to_geos(path, force_ccw=False): | |
geom = sgeom.LineString(path_verts) | ||
|
||
# If geom is a Polygon and is contained within the last geom in | ||
# collection, add it to its list of internal polygons, otherwise | ||
# simply append it as a new external geom. | ||
# collection, it usually needs to be an interior to that geom (e.g. a | ||
# lake within a land mass). Sometimes there is a further geom within | ||
# this interior (e.g. an island in a lake, or some instances of | ||
# contours). This needs to be a new external geom in the collection. | ||
if geom.is_empty: | ||
pass | ||
elif (len(collection) > 0 and | ||
isinstance(collection[-1][0], sgeom.Polygon) and | ||
isinstance(geom, sgeom.Polygon) and | ||
collection[-1][0].contains(geom.exterior)): | ||
collection[-1][1].append(geom.exterior) | ||
if any(internal.contains(geom) for internal in collection[-1][1]): | ||
collection.append((geom, [])) | ||
else: | ||
collection[-1][1].append(geom) | ||
elif isinstance(geom, sgeom.Point): | ||
other_result_geoms.append(geom) | ||
else: | ||
|
@@ -188,8 +193,8 @@ def path_to_geos(path, force_ccw=False): | |
geom_collection = [] | ||
for external_geom, internal_polys in collection: | ||
if internal_polys: | ||
# XXX worry about islands within lakes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume that |
||
geom = sgeom.Polygon(external_geom.exterior, internal_polys) | ||
exteriors = [geom.exterior for geom in internal_polys] | ||
geom = sgeom.Polygon(external_geom.exterior, exteriors) | ||
else: | ||
geom = external_geom | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I’ve tested this across quite a few plots, I’m now questioning whether it is quite right. If we append the geom from the third level down to
collection
, could the next geom be at the second level down and so would need to check against something earlier thancollection[-1]
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm... that is a good question. I wonder if you could somehow keep track of the "last" geometry in the nested collections and sort of recursively drill down/out from there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking of this as a tree problem with the path segments ordered in a "depth first" way. However looking at matplotlib/matplotlib#25247, I think the paths that used to be separate are now just concatenated together. So anything at the second level has to be directly preceded by either something else at the second level or something at the first level, and so my above hypothetical can't happen for contours at least.