JuliaGeo / GeoInterface.jl

A Julia Protocol for Geospatial Data
https://juliageo.org/GeoInterface.jl/stable/
MIT License
99 stars 17 forks source link

Should Multi types accept single geometries as inputs to make type promotion more intuitive #157

Closed alex-s-gardner closed 1 month ago

alex-s-gardner commented 1 month ago
import GeoInterface as GI
using DataFrames

r = 1000000;
ϴ = 0:0.01:2pi;
x = r .* cos.(ϴ) .^ 3 .+ 500000;
y = r .* sin.(ϴ) .^ 3 .+ 5000000;

ring3 = GI.LinearRing(GI.Point.(zip(x, y)))

# promote to polygon [DOES NOT WORK]
polygon = GI.Polygon(ring3)

# Instead I need to do this which is a bit awkward
polygon = GI.Polygon([ring3])

# create a dataframe with polygon geometires
df = DataFrame(geometry=[polygon, polygon, polygon, polygon])

# Now promote polygons to MultiPolygons [DOES NOT WORK]
df[!, :geometry] = GI.MultiPolygon.(df.geometry)

# Instead I need to do this which is a bit awkward
df[!, :geometry] = [GI.MultiPolygon([g]) for g in df.geometry]
rafaqz commented 1 month ago

Good idea, should be easy to do.

Just need one line that accepts the child type and wraps it in a vector and calls the constructor again. (It's all in an eval loop for every type at once)

alex-s-gardner commented 1 month ago

Haven't used eval before ... so something like this?

@eval function $geomtype{Z,M}(geom::$childtype; extent::E=nothing, crs::C=nothing) where {Z,M,E,C}
    $geomtype([geom]; extent, crs)
end
asinghvi17 commented 1 month ago

Fundamentally yes, but we don't have generic dispatch for child types. (Maybe we should?)

https://github.com/JuliaGeo/GeoInterface.jl/blob/b0156d3b2535dae6750bc3a50244a2136315acaf/src/wrappers.jl#L180-L184

is where you'd have to inject a check.

Instead of erroring if geomtrait(geom) isa $trait, you can check:

if geomtrait($geom) isa $trait
     Z1 = isnothing(Z) ? is3d(geom) : Z 
     M1 = isnothing(M) ? ismeasured(geom) : M 
     return $geomtype{Z1,M1,T,E,C}(geom, extent, crs) 
elseif geomtrait($geom) isa $childtrait 
    $geomtype{Z, M}([geom], extent, crs) # recurse so this hits the abstractarray dispatch
else
    _argument_error(T, $trait) 
end

(at least that's the logic I'd envision, can't guarantee this code is correct or free of syntax errors :D )

rafaqz commented 1 month ago

This is already how geometries are defined, you just need to find where to add that extra check.

Like here:

https://github.com/JuliaGeo/GeoInterface.jl/blob/main/src%2Fwrappers.jl#L187

asinghvi17 commented 1 month ago

Closed in #159