-
Notifications
You must be signed in to change notification settings - Fork 36
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
Allow empty coordinate arrays #100
Allow empty coordinate arrays #100
Conversation
@@ -207,6 +207,14 @@ def _wkt_coordinates(self) -> str: | |||
f"({_lines_wtk_coordinates(polygon)})" for polygon in self.coordinates | |||
) | |||
|
|||
@validator("coordinates") |
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.
In creating a test that should have failed I realized this wasn't actually constrained before and it should have been.
else: | ||
MultiPointCoords = conlist(Position, min_items=1) | ||
LineStringCoords = conlist(Position, min_items=2) |
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 tried to make these Annotated[List[Position], Field(min_items=2)]
which works for LineStringCoords
but does not properly constrain MultiLineStringCoords
. I think that is something that may get fixed in Pydantic 2.
A little surprised nothing complained about this in the type checking:
Other stuff:
|
"""Return the exterior Linear Ring of the polygon.""" | ||
return self.coordinates[0] | ||
return self.coordinates[0] if self.coordinates else None |
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.
The return type here is probably debatable. Since an exterior should be a LinearRing. But if there is no Polygon, is there any exterior?
I considered []
but:
LinearRing
has a min length of 4. The type checker doesn't actually know that, and will accept it, but it felt wrong.- There doesn't appear to really be a clean way to say a literal empty list as
Literal[[]]
is not valid.
Ended up going with None because it seemed the most clear.
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.
In [1]: from shapely.geometry import Polygon
In [2]: Polygon()
Out[2]: <POLYGON EMPTY>
In [3]: Polygon().interiors
Out[3]: []
In [4]: Polygon().exterior
Out[4]: <LINEARRING EMPTY>
shapely returns []
for interiors and return empty Linearring for exterior
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 did see that but a LINEARRING EMPTY
would not be valid GeoJSON since it states a LinearRing
has a min length of 4. Seemed like None
would be the closest thing.
The interiors function we have is a generator but list(Polygon(...).interiors)
matches the []
of shapely.
What I am changing
How I did it
conlist
from the types which are not constrained.LineString
andLinearRing
still have min items.How you can test it
Related Issues