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.

Sign up for GitHub

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

enable empty coordinantes #99

Closed
wants to merge 1 commit into from
Closed

enable empty coordinantes #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 63 additions & 58 deletions geojson_pydantic/geometries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import abc
from typing import Any, Dict, Iterator, List, Literal, Union

from pydantic import BaseModel, ValidationError, validator
from pydantic import BaseModel, Field, ValidationError, validator
from pydantic.error_wrappers import ErrorWrapper

from geojson_pydantic.types import (
Expand Down Expand Up @@ -36,7 +36,7 @@ class _GeometryBase(BaseModel, abc.ABC):
"""Base class for geometry models"""

type: str
coordinates: Any
coordinates: Union[Any, None] = Field(...)

@property
def __geo_interface__(self) -> Dict[str, Any]:
Expand All @@ -48,88 +48,81 @@ def __geo_interface__(self) -> Dict[str, Any]:

@property
@abc.abstractmethod
def _wkt_coordinates(self) -> str:
...

@property
@abc.abstractmethod
def _wkt_inset(self) -> str:
"""Return Z for 3 dimensional geometry or an empty string for 2 dimensions."""
...

@property
def _wkt_type(self) -> str:
"""Return the WKT name of the geometry."""
return self.type.upper()

@property
def wkt(self) -> str:
"""Return the Well Known Text representation."""
return self._wkt_type + (
f"{self._wkt_inset}({self._wkt_coordinates})"
if self.coordinates
else " EMPTY"
)
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the 2 abstract method because it was going to create more line of code (repeating the 2 if not self.coordinates)

Copy link
Collaborator

Choose a reason for hiding this comment

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

While there may need to be a couple extra safety checks to keep the type checking happy. Checking the value exists isn't an expensive operation but it cuts down on a lot of duplicated code to use the base functions.

I was actually going to propose a has_z property specifically to get rid of the _wkt_inset and get rid of the duplication of the " Z " if ... else " " everywhere.

...


class Point(_GeometryBase):
"""Point Model"""

type: Literal["Point"]
coordinates: Position
coordinates: Union[Position, None] = Field(...)

@property
def _wkt_coordinates(self) -> str:
return _position_wkt_coordinates(self.coordinates)
def wkt(self) -> str:
"""Return the Well Known Text representation."""
if not self.coordinates:
return f"{self.type.upper()} EMPTY"

@property
def _wkt_inset(self) -> str:
return " Z " if len(self.coordinates) == 3 else " "
wkt_inset = " Z " if len(self.coordinates) == 3 else " "
wkt_coordinates = _position_wkt_coordinates(self.coordinates)

return f"{self.type.upper()}{wkt_inset}({wkt_coordinates})"


class MultiPoint(_GeometryBase):
"""MultiPoint Model"""

type: Literal["MultiPoint"]
coordinates: MultiPointCoords
coordinates: Union[MultiPointCoords, None] = Field(...)

@property
def _wkt_inset(self) -> str:
return " Z " if len(self.coordinates[0]) == 3 else " "
def wkt(self) -> str:
"""Return the Well Known Text representation."""
if not self.coordinates:
return f"{self.type.upper()} EMPTY"

@property
def _wkt_coordinates(self) -> str:
return _position_list_wkt_coordinates(self.coordinates)
wkt_inset = " Z " if len(self.coordinates[0]) == 3 else " "
wkt_coordinates = _position_list_wkt_coordinates(self.coordinates)

return f"{self.type.upper()}{wkt_inset}({wkt_coordinates})"


class LineString(_GeometryBase):
"""LineString Model"""

type: Literal["LineString"]
coordinates: LineStringCoords
coordinates: Union[LineStringCoords, None] = Field(...)

@property
def _wkt_inset(self) -> str:
return " Z " if len(self.coordinates[0]) == 3 else " "
def wkt(self) -> str:
"""Return the Well Known Text representation."""
if not self.coordinates:
return f"{self.type.upper()} EMPTY"

@property
def _wkt_coordinates(self) -> str:
return _position_list_wkt_coordinates(self.coordinates)
wkt_inset = " Z " if len(self.coordinates[0]) == 3 else " "
wkt_coordinates = _position_list_wkt_coordinates(self.coordinates)

return f"{self.type.upper()}{wkt_inset}({wkt_coordinates})"


class MultiLineString(_GeometryBase):
"""MultiLineString Model"""

type: Literal["MultiLineString"]
coordinates: MultiLineStringCoords
coordinates: Union[MultiLineStringCoords, None] = Field(...)

@property
def _wkt_inset(self) -> str:
return " Z " if len(self.coordinates[0][0]) == 3 else " "
def wkt(self) -> str:
"""Return the Well Known Text representation."""
if not self.coordinates:
return f"{self.type.upper()} EMPTY"

@property
def _wkt_coordinates(self) -> str:
return _lines_wtk_coordinates(self.coordinates)
wkt_inset = " Z " if len(self.coordinates[0][0]) == 3 else " "
wkt_coordinates = _lines_wtk_coordinates(self.coordinates)

return f"{self.type.upper()}{wkt_inset}({wkt_coordinates})"


class LinearRingGeom(LineString):
Expand All @@ -148,7 +141,7 @@ class Polygon(_GeometryBase):
"""Polygon Model"""

type: Literal["Polygon"]
coordinates: PolygonCoords
coordinates: Union[PolygonCoords, None] = Field(...)

@validator("coordinates")
def check_closure(cls, coordinates: List) -> List:
Expand All @@ -161,22 +154,31 @@ def check_closure(cls, coordinates: List) -> List:
@property
def exterior(self) -> LinearRing:
"""Return the exterior Linear Ring of the polygon."""
if not self.coordinates:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Initially I thought this would be a None, as there is not an exterior at all. But I guess [] could be valid as well.

return []

return self.coordinates[0]

@property
def interiors(self) -> Iterator[LinearRing]:
"""Interiors (Holes) of the polygon."""
if not self.coordinates:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is necessary, as the current yield statement already covers this case.

return []

yield from (
interior for interior in self.coordinates[1:] if len(self.coordinates) > 1
)

@property
def _wkt_inset(self) -> str:
return " Z " if len(self.coordinates[0][0]) == 3 else " "
def wkt(self) -> str:
"""Return the Well Known Text representation."""
if not self.coordinates:
return f"{self.type.upper()} EMPTY"

@property
def _wkt_coordinates(self) -> str:
return _lines_wtk_coordinates(self.coordinates)
wkt_inset = " Z " if len(self.coordinates[0][0]) == 3 else " "
wkt_coordinates = _lines_wtk_coordinates(self.coordinates)

return f"{self.type.upper()}{wkt_inset}({wkt_coordinates})"

@classmethod
def from_bounds(
Expand All @@ -195,18 +197,21 @@ class MultiPolygon(_GeometryBase):
"""MultiPolygon Model"""

type: Literal["MultiPolygon"]
coordinates: MultiPolygonCoords
coordinates: Union[MultiPolygonCoords, None] = Field(...)

@property
def _wkt_inset(self) -> str:
return " Z " if len(self.coordinates[0][0][0]) == 3 else " "
def wkt(self) -> str:
"""Return the Well Known Text representation."""
if not self.coordinates:
return f"{self.type.upper()} EMPTY"

@property
def _wkt_coordinates(self) -> str:
return ",".join(
wkt_inset = " Z " if len(self.coordinates[0][0][0]) == 3 else " "
wkt_coordinates = ",".join(
f"({_lines_wtk_coordinates(polygon)})" for polygon in self.coordinates
)

return f"{self.type.upper()}{wkt_inset}({wkt_coordinates})"


Geometry = Union[Point, MultiPoint, LineString, MultiLineString, Polygon, MultiPolygon]

Expand Down
2 changes: 1 addition & 1 deletion tests/test_geometries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def test_line_string_valid_coordinates(coordinates):
assert_wkt_equivalence(linestring)


@pytest.mark.parametrize("coordinates", [None, "Foo", [], [(1.0, 2.0)], ["Foo", "Bar"]])
@pytest.mark.parametrize("coordinates", ["Foo", [(1.0, 2.0)], ["Foo", "Bar"]])
def test_line_string_invalid_coordinates(coordinates):
"""
But we don't accept non-list inputs, too few coordinates, or bogus coordinates
Expand Down