Skip to content

Commit

Permalink
Don't ignore invalid polygon shapes in geo interface
Browse files Browse the repository at this point in the history 
Interpret orphan holes as exteriors.
If only interior holes found, assume wrong winding order and return as exteriors.
  • Loading branch information
@karimbahgat
karimbahgat committed Aug 24, 2020
1 parent ad78236 commit e5407f3
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 17 deletions.
35 changes: 20 additions & 15 deletions shapefile.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def organize_polygon_rings(rings):
if bbox_overlap(hole_bbox, ext_bbox):
hole_exteriors[hole_i].append( ext_i )

# then, for holes with more than one possible exterior, do more detailed hole-in-ring test
# then, for holes with still more than one possible exterior, do more detailed hole-in-ring test
for hole_i,exterior_candidates in hole_exteriors.items():

if len(exterior_candidates) > 1:
Expand All @@ -305,37 +305,42 @@ def organize_polygon_rings(rings):
ext_i = sorted(exterior_candidates, key=lambda x: abs(signed_area(exteriors[x])))[0]
hole_exteriors[hole_i] = [ext_i]

# check for holes that are orphaned (not contained by any exterior)
orphan_holes = []
for hole_i,exterior_candidates in list(hole_exteriors.items()):
if not exterior_candidates:
warnings.warn('Shapefile shape has invalid polygon: found orphan hole (not contained by any of the exteriors); interpreting as exterior.')
orphan_holes.append( hole_i )
del hole_exteriors[hole_i]
continue

# each hole should now only belong to one exterior, group into exterior-holes polygons
polys = []
for ext_i,ext in enumerate(exteriors):
poly = [ext]
# find relevant holes
poly_holes = []
for hole_i,exterior_candidates in list(hole_exteriors.items()):
# ignore any hole that is orphaned (not contained by an exterior)
if not exterior_candidates:
warnings.warn('Shapefile shape has invalid polygon: found orphan hole (not contained by any of the exteriors); ignoring.')
del hole_exteriors[hole_i]
continue
# ignore any hole that is ambiguous (more than one possible exteriors)
# this shouldn't happen however, since ambiguous exteriors are resolved
# in the previous stage as the one with the smallest area.
if len(exterior_candidates) > 1:
warnings.warn('Algorithm error: algorithm was unable to resolve hole exterior among multiple possible candidates; ignoring.')
del hole_exteriors[hole_i]
continue

# hole is relevant if previously matched with this exterior
if exterior_candidates[0] == ext_i:
poly_holes.append( holes[hole_i] )
poly += poly_holes
polys.append(poly)

# add orphan holes as exteriors
for hole_i in orphan_holes:
poly = [holes[hole_i]]
polys.append(poly)

return polys

# no exteriors, bug?
else:
raise Exception('Shapefile shape has invalid polygon: no exterior rings found (must have clockwise orientation)')

warnings.warn('Shapefile shape has invalid polygon: no exterior rings found (must have clockwise orientation); interpreting holes as exteriors.')
exteriors = holes # could potentially reverse their order, but in geojson winding order doesn't matter
polys = [[ext] for ext in exteriors]
return polys

class Shape(object):
def __init__(self, shapeType=NULL, points=None, parts=None, partTypes=None):
Expand Down
43 changes: 41 additions & 2 deletions test_shapefile.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
# our imports
import shapefile

# define valid shape tuples of (type, points, parts indexes, and expected geo interface output)
# test all sorts, incl no holes, hole in singlepoly, hole in multipoly, nested holes and polys
# define various test shape tuples of (type, points, parts indexes, and expected geo interface output)
geo_interface_tests = [ (shapefile.POINT, # point
[(1,1)],
[],
Expand Down Expand Up @@ -127,6 +126,46 @@
],
]}
),
(shapefile.POLYGON, # multi polygon, holes incl orphaned holes (unordered), should raise warning
[(1,1),(1,9),(9,9),(9,1),(1,1), # exterior 1
(11,11),(11,19),(19,19),(19,11),(11,11), # exterior 2
(12,12),(14,12),(14,14),(12,14),(12,12), # hole 2.1
(15,15),(17,15),(17,17),(15,17),(15,15), # hole 2.2
(95,95),(97,95),(97,97),(95,97),(95,95), # hole x.1 (orphaned hole, should be interpreted as exterior)
(2,2),(4,2),(4,4),(2,4),(2,2), # hole 1.1
(5,5),(7,5),(7,7),(5,7),(5,5), # hole 1.2
],
[0,5,10,15,20,25,30],
{'type':'MultiPolygon','coordinates':[
[ # poly 1
[(1,1),(1,9),(9,9),(9,1),(1,1)], # exterior
[(2,2),(4,2),(4,4),(2,4),(2,2)], # hole 1
[(5,5),(7,5),(7,7),(5,7),(5,5)], # hole 2
],
[ # poly 2
[(11,11),(11,19),(19,19),(19,11),(11,11)], # exterior
[(12,12),(14,12),(14,14),(12,14),(12,12)], # hole 1
[(15,15),(17,15),(17,17),(15,17),(15,15)], # hole 2
],
[ # poly 3 (orphaned hole)
[(95,95),(97,95),(97,97),(95,97),(95,95)], # exterior
],
]}
),
(shapefile.POLYGON, # multi polygon, exteriors with wrong orientation (be nice and interpret as such)
[(1,1),(9,1),(9,9),(1,9),(1,1), # exterior with hole-orientation
(11,11),(19,11),(19,19),(11,19),(11,11), # exterior with hole-orientation
],
[0,5],
{'type':'MultiPolygon','coordinates':[
[ # poly 1
[(1,1),(9,1),(9,9),(1,9),(1,1)],
],
[ # poly 2
[(11,11),(19,11),(19,19),(11,19),(11,11)],
],
]}
),
]

def test_empty_shape_geo_interface():
Expand Down

0 comments on commit e5407f3

Please sign in to comment.