Return extent based on dimensionality.

[evetion]

Before we always returned the 3d extent, even on 2d geometries.

[rafaqz]

Lol I have an unfinished PR for this too.

[rafaqz]

It should be possible to make this type stable? It could use is3d, which could also be type stable.

[evetion]

As in having two functions and passing Val(is3d()) right?

[rafaqz]

That won't be type stable cos is3d is currently runtime.

I mean make is3d depend on the type instead of on a GDAL function. Then it will constant prop at compile time without using Val.

[evetion]

Ok, will do, but then we need to wait on #316.

[rafaqz]

We don't need to wait tho it doesn't change the types

[evetion]

Your PR introduces the AbstractGeometry{T} right? So

function GeoInterface.is3d(
        ::GeometryTraits,
        geom::Union{map(T -> AbstractGeometry{T}, ztypes)...},
    )
        return true
    end

would require #316

[rafaqz]

Yeah you're right. My other PR for this just used a Union in the geometry types before I just changed the abstract types.

I might add is3d to the other PR as it falls under type stability improvements. Then this PR can just call is3d.

Edit: I guess even getcoorddim can be a compile time trait. In which case this PR is fine as-is.

[evetion]

To be fair, I would opt for using GI.is3d here. I like to make all GeoInterface methods compile time based and leave the ArchGDAL functions calling GDAL as is.

[rafaqz]

Yeah that sounds like a good idea.

But also it turns out to be not so easy to do this at all because there is no wkbPointZ. So out points are type unstable between 2/3 tuples. We probably need to ecode the dimensionality as an additional parameter in the point type.

[evetion]

There's wkbPoint25D right? It's confusing, I agree.

[rafaqz]

Yeah but no Z when there is ZM? I cant tell how its all mean to work.

[evetion]

wkbPoint =>    X Y 
wkbPoint25D => X Y Z
wkbPointM =>   X Y M
wkbPointZM =>  X Y Z M

And to make it more confusing, for the extended geometries, they didn't use 25D, but Z again (wkbCircularStringZ to wkbTriangleZ)

[rafaqz]

Yeah thats what threw me... there are Z geometries and 25D geometries.

[evetion]

If @rafaqz agrees, this can now be merged as is. is3d can be sped up in the other PR(s).

[rafaqz]

Yep let's do that.

[evetion]

Note that I can't merge because we need at least 1 reviewer approving ;)

[rafaqz]

LGTM

[yeesian]

Note that I can't merge because we need at least 1 reviewer approving ;)

Sorry I set it up per https://github.com/SciML/ColPrac/blob/b6cacb2942753a0bb6b721c81543b649f9765063/README.md?plain=1#L29

@rafaqz FYI I think you should have the requisite permissions to approve PRs through the github UI too :)

[yeesian]

Thank you for this!