move to GeometryBasics #39
Conversation
| x::Float64 | ||
| y::Float64 | ||
| end | ||
| const Point = GB.Point{2, Float64} |
There was a problem hiding this comment.
should this be Pointe0 instead? (a la Pointf0)
There was a problem hiding this comment.
Ha yeah, though then it should also include the 2 I guess.
I think for now it is easiest to keep the names the same as the original structs though. They won't be exported anyway, only used as internal aliases.
There was a problem hiding this comment.
Yeah, if we do anything, this shoulg go into GeometryBascs!
There was a problem hiding this comment.
Although, Point0 would be super ambigious.. I guess we'd rather go for something like Point2d for double... Although that's also ambigious, but at least doesnt look as weird^^
Add types anfd read methods for PolygonM, PolygonZ
Add types and fix read methods for PolylineM, PolylineZ
shapes now has a type
overwrite parts values according to 1 based indexing Add Polyline alias, fix return type Add types and fix read methods for PolylineM, PolylineZ Create type and fix read method for MultiPatch Polygon parts(heisenbug) Input exterior and interiors to Polygon Refactor polygon, rectify parts for polyline
|
why don't you want to have it as a dep? It has no dependencies itself, and it really defines only two functions. If we copy the |
|
I will extend my comment above into a PR against this PR tomorrow |
|
Hmm yeah good point. I see now PolygonOps is pretty minimal, so I guess it's fine. Still have to get to terms with the fact that we need to do polygon ops to parse polygons... Not sure how the PolygonOps and shapefile.js algorithms compare though. The JavaScript looks even more compact. |
This goes back to the confusion about the definition of polynomials on Slack. PolygonOps is not about So probably it would be nice to have a |
|
Yeah, right now I'd keep things somewhat close to the Simple Features standard, and would rather use
The LinearRing comes back much more, see for instance: In terms of representation a LineString and LinearRing would be much the same. We probably need to decide if we also want to have the same point as first and last point just like in the specs, see for instance JuliaGeometry/GeometryBasics.jl#9 |
Read polygons to GB.MultiPolygon
Remove GI
| mmin = read(io, Float64) | ||
| mmax = read(io, Float64) | ||
| jltype = SHAPETYPE[shapeType] | ||
| shapes = Vector{Union{jltype,Missing}}(undef, 0) |
There was a problem hiding this comment.
If anyone wants to try this:
I temporarily changed this shapes = Any[]. With this change I can use the package to read all kinds of maps. After this was reverted in the last commit by @Sov-trotter this branch is not usable for me.
@Sov-trotter, unless you have a good reason, it would be nice if you could remove your commit, so that I can use this branch instead of my own.
The error I am seeing is this one
MethodError: Cannot `convert` an object of type
GeometryBasics.MultiPolygonMeta{GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},L,V} where V<:AbstractArray{L,1} where L<:(AbstractArray{var"#s32",1} where var"#s32"<:GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}}),GeometryBasics.MultiPolygon{2,Float64{},GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},L,V} where V<:AbstractArray{L,1} where L<:(AbstractArray{var"#s32",1} where var"#s32"<:GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}}),Array{GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},L,V} where V<:AbstractArray{L,1} where L<:(AbstractArray{var"#s32",1} where var"#s32"<:GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}}),1}},(:boundingbox,),Tuple{Shapefile.Rect{}}} to an object of type
GeometryBasics.MultiPolygonMeta{GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},Array{GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},1}},GeometryBasics.MultiPolygon{2,Float64{},GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},Array{GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},1}},Array{GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},Array{GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},1}},1}},(:boundingbox,),Tuple{Shapefile.Rect{}}}
Closest candidates are:
convert(::Type{T}, !Matched::T) where T<:AbstractArray at abstractarray.jl:14
convert(::Type{T}, !Matched::LinearAlgebra.Factorization) where T<:AbstractArray at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.5/LinearAlgebra/src/factorization.jl:55
convert(::Type{S}, !Matched::T) where {S, T<:CategoricalValue} at /Users/fabiangreimel/.julia/packages/CategoricalArrays/0ZAbp/src/value.jl:68
...
push!(::Array{GeometryBasics.MultiPolygonMeta{GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},Array{GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Point{2,Float64}},1,Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2,Float64},GeometryBasics.Point{2,Float64}},2,1,Array{GeometryBasics.Point{2,Float64},1}}}},1}},GeometryBasics.MultiPolygon{2,Float64,GeometryBasics.Polygon{2,Float64,GeometryBasics.Point{2,Float64},GeometryBasics.LineString{2,Float64,GeometryBasics.Point{2,Float64},Base.ReinterpretArray{GeometryBasics.Ngon{2,Float64,2,GeometryBasics.Poin...
There was a problem hiding this comment.
Oh. The git stuff on my local side is also a bit messy. I think a cleaner option here would be, branching from here? And make the changes accordingly in a new pr?
Sov-trotter
force-pushed
the
sd/geometrybasics
branch
from
5688205 to
b709412
Compare
Reset stuff
|
Just so that it doesn't get lost. What is to be done here (because it was missing from my PR #40) is
this is probably material for a separate PR against this branch, but I don't have time for this in the next months. |
|
Is there any chance we can get this into a mergeable state? |
|
@greimel is the current state of |
|
I've been using this branch for reading many shapefiles for many countries without problems. So I think it's mergeable.
I've not yet encountered incorrectly specified shapefiles, so I couldn't justify working on that.
I don't remember why this is going wrong. But I think the problem is that the different MultiPolynomials had different types if they have holes or not or if they consist of multiple segments, so we cannot determine the type in advance. I wondered if using https://juliafolds.github.io/BangBang.jl/v0.1/#BangBang.push!!-Tuple{Any,Any,Any,Vararg{Any,N}%20where%20N} would be acceptable here? |
|
As an aside, we should add a map of https://en.wikipedia.org/wiki/Baarle-Hertog to the tests. If we can handle this, what else could go wrong. |
|
@greimel can you share that shapefile you're trying to read which gives an error due to |
This PR is aimed at providing GeometryBasics types for Shapefile geometries. One of the major advantages being it's Metadata capability and the ability to plot Shapefiles with Makie/GeoMakie where the former already supports GeometryBasic types.
It's an ecosystem wide movement to move more to GeometryBasics.
It is planned to be merged along with GeoInterfaceRFC once both are ready.