some editing of #33

[visr]

Nothing major, but I'm making some edits as I go over the current status of the #33 branch (which looks quite good overall!).

[visr]

One thing I want to make sure, in the test implementation there was a

GeoInterface.convert(::Type{MyCurve}, ::GeoInterface.LineStringTrait, geom) = geom

That should be a Base.convert right? That is what I did here.

[evetion]

One thing I want to make sure, in the test implementation there was a

GeoInterface.convert(::Type{MyCurve}, ::GeoInterface.LineStringTrait, geom) = geom

That should be a Base.convert right? That is what I did here.

Probably not. I struggled with this myself, but we cannot define it here without overriding Base.convert(::Any, ::Any). I'm not sure if that's something we want. Then again, it would work better if it did though?

@rafaqz

[rafaqz]

Yeah that's a little tricky. Can we internally do:

Base.convert(::Type, ::AbstractGeometryTrait, geom) = error("some comvert error asking for a method to be defined")

And just get packages to define:

Base.convert(::Type{T}, geom) where T<:AbstractPackageType = Base.convert(T, geomtype(geom), geom)

(or make another macro like with plots)

[evetion]

@rafaqz @visr
I've implemented the suggestion above in 2cc0e07 (#33). One tricky thing, convert is also called in the iteration we provide ourselve (like getgeom). So we need to provide another method:

Base.convert(::Type{X}, ::LineStringTrait, geom::X) = geom  # fast fallthrough without conversion    

Otherwise the convert will handle the geom as if it's another geometry altogether, getting coordinates, making copies, etc.

[visr]

I see now only Base.convert in the docs: https://github.com/JuliaGeo/GeoInterface.jl/blob/e34d37e59cda58a9b0adbe98e95f593a8cd29d35/docs/src/guides/developer.md#conversion

Does that mean that these can be removed?

GeoInterface.jl/src/interface.jl

Line 564 in e34d37e

convert(T, geom) = convert(T, geomtrait(geom), geom)

GeoInterface.jl/GeoInterfaceRecipes/test/runtests.jl

Line 32 in 4abac25

GeoInterface.convert(::Type{MyCurve}, ::GeoInterface.LineStringTrait, geom) = geom