-
-
Notifications
You must be signed in to change notification settings - Fork 261
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 __geo_interface__ if parts or points is empty list #101
Conversation
|
||
""" Case parts or points is empty """ | ||
if not self.parts or not self.points: | ||
return False |
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.
False? I am not sure about this. how does geojson handle empty geometries?
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.
False is a valid value for geojson data structure. (https://geojson.org/geojson-spec.html#geometry-objects) I did some tests with geojson that contains a false value in geometry fields in GIS tools, and i not found erros.
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.
There doesn't seem to be an empty geometry type defined in GeoJSON, but it does state that feature objects without a geometry can be specified with JSON null value, which would be None rather than False in Python.
From the previous link:
A feature object must have a member with the name "geometry". The value of the geometry member is a geometry object as defined above or a JSON null value.
I also think None makes more sense since there literally is "none" geometry object.
Thanks for this @Tisp. However, I believe the unbound local error happens for good reasons. Having geo_interface return False or None would imply that this is a null-geometry. But geo_interface is already built to return None when the shape type is set to NULL. This means your code was processing a real geometry, specifically a polyline or polygon geometry which should always contain a So while the error message is being raised correctly it is not very informative. Perhaps instead of returning False we can return a more informative exception if the parts attribute is empty? Furthermore, this should only be checked for polyline or polygon types, as these are the only ones expected to have the |
I agree. Right now I'm handling the |
Fixed this now, raises a generic Exception when this happens (but see #146). Also raises error if shape type is beyond what the GeoJSON specification can represent. |
Fixes UnboundLocalError when shapes have empty parts or points
File "/usr/local/lib/python2.7/site-packages/shapefile.py", line 174, in __geo_interface__ coordinates.append(tuple([tuple(p) for p in self.points[part:]])) UnboundLocalError: local variable 'part' referenced before assignment