Refactor WKT for reuse and speed. #97
Conversation
| @@ -50,7 +65,11 @@ def _wkt_type(self) -> str: | |||
| @property | |||
| def wkt(self) -> str: | |||
| """Return the Well Known Text representation.""" | |||
| return f"{self._wkt_type}{self._wkt_inset}({self._wkt_coordinates})" | |||
| return self._wkt_type + ( | |||
There was a problem hiding this comment.
I guess this change didn't have to be in this PR, since self.coordinates cannot be None yet.
| @@ -22,7 +22,7 @@ def assert_wkt_equivalence(geom: Union[Geometry, GeometryCollection]): | |||
| """Assert WKT equivalence with Shapely.""" | |||
| # Remove any trailing `.0` to match Shapely format | |||
| clean_wkt = re.sub(r"\.0(\D)", r"\1", geom.wkt) | |||
| assert shape(geom.dict()).wkt == clean_wkt | |||
| assert shape(geom).wkt == clean_wkt | |||
There was a problem hiding this comment.
shape uses the __geo_interface__ to construct so don't need .dict(). This also tests our implementation is usable by shapely.
| [[(1.0, 2.0), (3.0, 4.0), (5.0, 6.0)]], | ||
| # Two lines, two points each, no Z |
There was a problem hiding this comment.
I believe there was a mismatch not being caught without these tests. There was a join with just comma and not comma + space.
Let's open new issues for those case. As far I'm concerned, this PR is good to go. @eseglem I'll let you set it to |
What I am changing
pydocstylewhich changed and threw a bunch of errors.How I did it
How you can test it
Related Issues
This doesn't address one issue I noticed while testing: mixed dimensionality. Shapely doesn't allow for it. So a
MultiPoint(coordinates=[(0,0),(1,1,1)])will becomeMULTIPOINT Z (0 0 0, 1 1 1)but we do not enforce that. Not sure where to go with that.I would also like to use hypothesis to generate tests with the pydantic hypothesis plugin rather than having so many hard coded hard to read values. It would likely break for the above case, but it would be good to know more about where things are different.