convert to module types

[rafaqz]

This PR adds the method:

GeoInterface.convert(Package, geom)

I still need to add some tests with a dummy module.

@evetion and @visr what do you think of this traittype as the interface method that returns a geometry/feature type for a corresponding trait?

As we have discussed packages will only have to define that method, e.g. in GeoJSON is all that is needed for
convert(GeoJSON, geom) to work (along with the actual basic convert methods I'm also adding now):

GI.traittype(::Val{:GeoJSON}, ::GI.PointTrait) = Point
GI.traittype(::Val{:GeoJSON}, ::GI.LineStringTrait) = LineString
GI.traittype(::Val{:GeoJSON}, ::GI.PolygonTrait) = Polygon
GI.traittype(::Val{:GeoJSON}, ::GI.MultiLineStringTrait) = MultiLineString
GI.traittype(::Val{:GeoJSON}, ::GI.MultiPolygonTrait) = MultiPolygon
GI.traittype(::Val{:GeoJSON}, ::GI.GeometryCollectionTrait) = GeometryCollection

[visr]

This looks like a nice solution to me.

So the only thing that traittype is used for is to provide the T in GeoInterface.convert(T, x), right? Or did you have other future uses in mind already?

What about packages that offer multiple point types, e.g. PointA and PointB? T can be abstract as well, e.g. convert(Integer, 2.0) works. So if there is an abstract point type then the convert method can decide which of the two point types to create. So that seems fine to me.

This also still needs docs.

Also: fixes #80

[rafaqz]

Yeah mostly just the for that T. It should also be useful internally in the packages that define it - there are const lookups doing similar things in some packages. Having everything package doing that in the same way is also a bonus.

Another option is to make it an agreed apon package method instead of a shared interface:

(In GeoJSON.jl)

traittype(::GI.PointTrait) = Point
traittype(::GI.LineStringTrait) = LineString
traittype(::GI.PolygonTrait) = Polygon
traittype(::GI.MultiLineStringTrait) = MultiLineString
traittype(::GI.MultiPolygonTrait) = MultiPolygon
traittype(::GI.GeometryCollectionTrait) = GeometryCollection

and here just do:

function convert(package::Module, geom)
    convert(package.traittype(package, t), t, geom)
end

Which seems kind of cleaner actually, and doesn't have the module name sharing problem.

And yes the abstract type thing can just be worked out by the package convert methods. Sometimes they will need to call is3d and ismeasured on the geom object to resolve what the final type will be. There may be some cases where traittype is not enough, but it should get most of the way.

[visr]

That does look better indeed. So how do we proceed?

I think #66 can already be merged and is not breaking right? And we can at any point already start moving implementations over from Base to GeoInterface convert, or we wait for this to be tagged and also add in the convert to Module methods from this PR directly. For packages moving from Base to GeoInterface convert is breaking however.

GeometryBasics already uses GeoInterface.convert, but ArchGDAL and LibGEOS still use Base.convert. I haven't checked others yet.

[rafaqz]

Yes I think this is better. I've written the GeoJSON.jl implementation.

ArchGDAL will likely have to return a single abstract type from traittype and work out the rest from the geom object in its own GeoInterface.covert method after dispatch.

Yes let's merge #66

Edit: this is updated to be my second proposal, with traittype defined per module.

I'm wondering if the function should be called geointerface_trait_type now?

[rafaqz]

Added a test - this should be good to merge.

[evetion]

LGTM if you fix the tests ;)

nitpick: geointerface_traittype(trait) should be the inverse of (geom)trait(geom). So maybe geointerface_(geom)type(trait)?

[rafaqz]

I'm a bit confused by your ( in the name change proposal, what should the name be??

[evetion]

I'm a bit confused by your ( in the name change proposal, what should the name be??

Ah, that was because we both have geomtrait(geom) as trait(geom). The latter being more generic, also working for feature.

So the inverse of trait(thing::T) would be thing(trait:T)=T() or thingtype(trait:T)=T. With the required namespacing and assuming geometries, you get either geointerface_geom(trait) or geointerface_geomtype(trait)?

But this is all semantics, I'm up for merging as is.

[rafaqz]

Renamed to geointerface_geomtype, you're right its better.