JuliaGeometry / GeometryBasics.jl

Basic Geometry Types
MIT License
165 stars 54 forks source link

vectors are not `isgeometry` material #193

Closed rafaqz closed 1 year ago

rafaqz commented 1 year ago

Vectors of geometries were defined as geometries here. But that breaks heaps of the interface as they don't actually return a geomtrait.

So removing. No tests as its just deleting things that are bugs.

Another tiny change is using AbstractMesh in isgeometry instead of Mesh because geomtrait uses AbstractMesh.

rafaqz commented 1 year ago

@visr or @SimonDanisch please approve my workflow :pleading_face:

Or maybe add me as a member here or in JuliaGeometry? this wont be the last PR...

visr commented 1 year ago

I approved the workflow but I cannot add you myself.

Note that this breaks isgeometry for the Multi geometries, which are not AbstractGeometry, but AbstractVector.

using GeoInterface, GeometryBasics
mp = MultiPoint([Point(3, 1)])
GeoInterface.isgeometry(mp)  # was true, now becomes false
rafaqz commented 1 year ago

Ahh right it was using AbstractArray as a catch all for those. I'll add them explicitly

visr commented 1 year ago

Right, that works.

Tests still fail, now with UndefVarError: AbstractLineString not defined

rafaqz commented 1 year ago

Yeah sorry I didn't mean to push and trigger that again.

I ended up having to fix a lot of things for the latest GeoJSON to work, as its now returning Float32 - but GeometryBasics should be able to use whatever type it gets. The modified convert methods should be faster anyway because of the branches.

I also added a few more missing tests for the interface while I was there, it's turned into a grab bag of things by now...

Tests are passing for me locally now anyway.

visr commented 1 year ago

Should #192 be closed in favor of this now?

rafaqz commented 1 year ago

Oh right I thought I had dejavu for those bits. Yes this implementation is better anyway.

Oh but there are a bunch of things in there not here - these were more because I had to change it to make the type work anyway.