yeesian / ArchGDAL.jl

A high level API for GDAL - Geospatial Data Abstraction Library
https://yeesian.github.io/ArchGDAL.jl/stable/
Other
142 stars 26 forks source link

Can't create geometry with non-Float64 vectors #304

Closed evetion closed 2 years ago

evetion commented 2 years ago

Encountered in #290, with creation of geometry from GeoInterface.coordinates(geom), which are nested Vectors of <:Real.

While you can create points with Integers:

julia> import ArchGDAL as AG
julia> AG.createpoint([1,2])
Geometry: POINT (1 2)

You cannot do so with more complex geometries, even with LineString:

julia> AG.createlinestring([[1,2],[1,2]])
ERROR: MethodError: no method matching createlinestring(::Vector{Vector{Int64}})

Not even with other Float types:

julia> AG.createlinestring([Float32.([1,2]),Float32.([1,2])])
ERROR: MethodError: no method matching createlinestring(::Vector{Vector{Float32}})
rafaqz commented 2 years ago

This line is what limits createlinestring to Float64: https://github.com/yeesian/ArchGDAL.jl/blob/e401e0079d97dac9d4755a4bc9dce345a9c5bf81/src/ogr/geometry.jl#L1519

(is this a real limitation of GDAL or something we have imposed unnecessarily?)

While point uses Real: https://github.com/yeesian/ArchGDAL.jl/blob/e401e0079d97dac9d4755a4bc9dce345a9c5bf81/src/ogr/geometry.jl#L1580

This code is all pretty hard to understand though: https://github.com/yeesian/ArchGDAL.jl/blob/e401e0079d97dac9d4755a4bc9dce345a9c5bf81/src/ogr/geometry.jl#L1484-L1675

Another thing that would be good to break out into named lists that we then loop over, so the code is a little more self-documenting.

rafaqz commented 2 years ago

Maybe this is an unnessesary limitation, swapping to <:Real seems to work fine.

Seeing its defined in multiple places, something like this could help:

const _VECTYPES = Vector{<:Tuple{<:Real,<:Real}}}},
                Vector{<:Tuple{<:Real,<:Real,<:Real}}}},
                Vector{<:Vector{<:Real}}}}
yeesian commented 2 years ago

I don't think it's due to any fundamental GDAL reason, and is probably me not being sufficiently clever to think about how to write it to cover all cases.

rafaqz commented 2 years ago

I'll PR when I have time, its an easy fix.