Why support WKT?

[eseglem]

I was working on a fix for #89 and it broke the WKT implementation and corresponding tests. I went down the rabbit hole and started refactoring all of it so it would work better and cleaner but it got me thinking: Why does geojson-pydantic support WKT?

geojson_pydantic provides a suite of Pydantic models matching the GeoJSON specification rfc7946. Those models can be used for creating or validating geojson data.

It is a library for creating and validating GeoJSON based on the spec. And directly supporting WKT does not serve that purpose. I checked and https://github.com/jazzband/geojson and it does not directly support WKT either. Why should a GeoJSON library support anything other than GeoJSON and __geo_interface__? I don't think anyone is going to argue that we should directly support reading shapefiles or anything like that. So why WKT?

Since the library already supports __geo_interface__ (although we could improve it) that should be plenty. Using shapely lets you get in and out of WKT or any other format it recognizes via __geo_interface__ and it feels like that is where the focus should stay. The tests around WKT are already using this to make sure our code matches what shapely outputs anyways. Why rebuild something they already do really well?

from shapely.geometry import shape
from geojson_pydantic import Point

shape(Point(type="Point", coordinates=(1,1))).wkt
# 'POINT (1 1)'

Arguments against keeping WKT:

• It isn't a part of the GeoJSON RFC.
• Shapely already does WKT really well, and we have to spend effort keeping things in line with it when that effort could be spent elsewhere (like __geo_interface__).
• Its probably close to half of the code in geometries.py and we could reduce the complexity of it as well as the corresponding tests.
• It needs a big refactor as they are extremely inefficient right now.

Arguments for keeping WKT:

• We already have it, and it would be a loss of a feature someone wanted enough to add.
• I have already done most of the work for refactoring it.
• ?

And don't get me wrong. It is convenient, even nice to have in some cases, but it also seems like its outside the scope of the library.

Ping @vincentsarago @geospatial-jeff

[vincentsarago]

@eseglem thanks for working on this.

As I'm the one who added WKT, I will argue that it's a really useful feature which at the time didn't look like something that would requirement too much work and burden to maintain.

In a sense, you can export a model to dict and json so why not wkt 🤷‍♂️

Using shapely lets you get in and out of WKT or any other format it recognizes via geo_interface and it feels like that is where the focus should stay. The tests around WKT are already using this to make sure our code matches what shapely outputs anyways. Why rebuild something they already do really well?

Shapely is a HUGE library which comes with some C lib, geojson-pydantic has only pydantic has requirement so it's relatively lightweight which make have a WKT feature really interesting.

[vincentsarago]

for empty geometry the wkt property should just be like ?

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

        return f"{self._wkt_type}{self._wkt_inset}({self._wkt_coordinates})"

[eseglem]

What I have in my local branch currently is:

    @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"
        )

Yours is probably easier to read though.

[eseglem]

And honestly, I don't have particularly strong feelings about it either way. But I figured it was worth asking before I refactored my refactor and submit a PR over it.

Shapely definitely is a big library, and not wanting to require it is certainly reasonable.