-
Notifications
You must be signed in to change notification settings - Fork 27
GeoJSON to 0.6 and GeoInterface support #123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GeoJSON to 0.6 and GeoInterface support #123
Conversation
Thanks for taking this on! I think something like The |
The integer issue occurs in both package datasets (search for One line example of the problem is here, when the patch is commented out (look at the final element): using GeoInterface, GeoMakie
GeoInterface.coordinates(GeoInterface.geometry.(GeoMakie.coastlines())[94]) Without the fix / hack, you get the following error: ERROR: MethodError: no method matching GeometryBasics.Polytope(::Type{GeometryBasics.Ngon{Dim, T, 2, P} where {Dim, T, P<:GeometryBasics.AbstractPoint{Dim, T}}}, ::Type{Point2})
Closest candidates are:
GeometryBasics.Polytope(::Type{<:GeometryBasics.Ngon{Dim, T, N, P} where {Dim, T, P}}, ::Type{<:GeometryBasics.AbstractPoint{NDim, T}}) where {N, NDim, T} at ~/.julia/packages/GeometryBasics/5Sb5M/src/basic_types.jl:89
Stacktrace:
[1] connect
@ ~/.julia/packages/GeometryBasics/5Sb5M/src/viewtypes.jl:77 [inlined]
[2] GeometryBasics.LineString(points::Vector{Point2}, skip::Int64) (repeats 2 times)
@ GeometryBasics ~/.julia/packages/GeometryBasics/5Sb5M/src/basic_types.jl:209
[3] convert(#unused#::Type{GeometryBasics.LineString}, type::GeoInterface.LineStringTrait, geom::GeoJSON.LineString{JSON3.Object{Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}}})
@ GeometryBasics ~/.julia/packages/GeometryBasics/5Sb5M/src/geointerface.jl:88
[4] convert
@ ~/.julia/packages/GeoInterface/J298z/src/interface.jl:612 [inlined]
[5] geoJSONtraitParse(geometry::GeoJSON.LineString{JSON3.Object{Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}}})
@ GeoMakie ~/.julia/dev/GeoMakie/src/patch.jl:26
[6] _broadcast_getindex_evalf
@ ./broadcast.jl:670 [inlined]
[7] _broadcast_getindex
@ ./broadcast.jl:643 [inlined]
[8] getindex
@ ./broadcast.jl:597 [inlined]
[9] copyto_nonleaf!(dest::Vector{GeometryBasics.LineString{2, Float64, Point2{Float64}, Base.ReinterpretArray{GeometryBasics.Line{2, Float64}, 1, Tuple{Point2{Float64}, Point2{Float64}}, GeometryBasics.TupleView{Tuple{Point2{Float64}, Point2{Float64}}, 2, 1, Vector{Point2{Float64}}}, false}}}, bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Tuple{Base.OneTo{Int64}}, typeof(GeoMakie.geoJSONtraitParse), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(GeoInterface.geometry), Tuple{Base.Broadcast.Extruded{GeoJSON.FeatureCollection{GeoJSON.Feature{JSON3.Object{Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}}}, JSON3.Object{Base.CodeUnits{UInt8, String}, Vector{UInt64}}, JSON3.Array{JSON3.Object, Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}}}, Tuple{Bool}, Tuple{Int64}}}}}}, iter::Base.OneTo{Int64}, state::Int64, count::Int64)
@ Base.Broadcast ./broadcast.jl:1055
[10] copy
@ ./broadcast.jl:907 [inlined]
[11] materialize(bc::Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(GeoMakie.geoJSONtraitParse), Tuple{Base.Broadcast.Broadcasted{Base.Broadcast.DefaultArrayStyle{1}, Nothing, typeof(GeoInterface.geometry), Tuple{GeoJSON.FeatureCollection{GeoJSON.Feature{JSON3.Object{Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}}}, JSON3.Object{Base.CodeUnits{UInt8, String}, Vector{UInt64}}, JSON3.Array{JSON3.Object, Base.CodeUnits{UInt8, String}, SubArray{UInt64, 1, Vector{UInt64}, Tuple{UnitRange{Int64}}, true}}}}}}})
@ Base.Broadcast ./broadcast.jl:860
[12] top-level scope
@ REPL[2]:1 |
Hmm you're right, it seems like the integer issue is quite common, since for every single point there is a risk of it happening, especially as you say with few decimals. |
Weird. I'm also on a M1 machine (and not crashing or stalling ) and the following versions work ok for Julia 1.8 (ARM (M-series Processor))
|
Thanks for the heads up @lazarusA Just retried on my machine. Still stalls at julia> versioninfo()
Julia Version 1.8.0
Commit 5544a0fab76 (2022-08-17 13:38 UTC)
Platform Info:
OS: macOS (arm64-apple-darwin21.3.0)
CPU: 10 × Apple M1 Pro
WORD_SIZE: 64
LIBM: libopenlibm
LLVM: libLLVM-13.0.1 (ORCJIT, apple-m1)
Threads: 1 on 8 virtual cores (@v1.8) pkg> st
Status `~/.julia/environments/v1.8/Project.toml`
[13f3f980] CairoMakie v0.8.13
[8be319e6] Chain v0.5.0
⌅ [61d90e0f] GeoJSON v0.5.1
[db073c08] GeoMakie v0.4.2
[c3e4b0f8] Pluto v0.19.11
[c94c279d] Proj v1.0.0 |
Yes, having an update will be nice. BTW. This is the env that is working for me: julia> versioninfo()
Julia Version 1.8.0
Commit 5544a0fab76 (2022-08-17 13:38 UTC)
Platform Info:
OS: macOS (arm64-apple-darwin21.3.0)
CPU: 8 × Apple M1
WORD_SIZE: 64
LIBM: libopenlibm
LLVM: libLLVM-13.0.1 (ORCJIT, apple-m1)
Threads: 6 on 4 virtual cores
Environment:
JULIA_EDITOR = code
JULIA_NUM_THREADS = 6
(map_projections) pkg> st
Status `~/Desktop/Julia1dot8/Project.toml`
[13f3f980] CairoMakie v0.8.13
[e9467ef8] GLMakie v0.6.13
⌅ [61d90e0f] GeoJSON v0.5.1
[db073c08] GeoMakie v0.4.2
[3cb90ccd] RasterDataSources v0.5.4
⌃ [a3a2b9e3] Rasters v0.2.2
[ade2ca70] Dates
Info Packages marked with ⌃ and ⌅ have new versions available, but those with ⌅ cannot be upgraded. To see why use `status --outdated` |
it looks like this PR is ready @asinghvi17, isn't? |
Hey @asinghvi17 @SimonDanisch!
Thanks for the great package. GeoJSON 0.5 was crashing / stalling on my M1 machine (( believe due to an outdated / nonfunctional binary dependency), so I went down the rabbit hole of updating
GeoMakie
to supportGeoJSON
0.6. There is a set of hacks insrc/patch.jl
and modifications to example plots which are the result of my limited understanding of theGeoInterface
/GeometryBasics
ecosystem, but the tests pass and I would be happy to put more time into eliminating the hacks and rolling back the example plot modifications if you could point me in the right direction.Ideally, when finished, this PR will resolve #117.
Cheers, Jeremiah