JuliaEarth / geospatial-data-science-with-julia

Geospatial Data Science with Julia
https://juliaearth.github.io/geospatial-data-science-with-julia
87 stars 16 forks source link

Comment regarding GeoInterface #4

Closed evetion closed 11 months ago

evetion commented 11 months ago

Congratulations on the book, it's looking very good! I'm looking forward to the sections still in progress.

While skimming along, I encountered the following in 3.3:

If we treated this geometry as a generic polygon represented by a vector of vertices in memory, like it is done in GeoInterface.jl for example, we wouldn’t be able to dispatch optimized code that is only valid for a triangle:

I agree with the overall point, generic representations of polygons from GIS are bad for any optimization. But I don't think the example make sense here, because GeoInterface can actually do what you want here, as it has a TriangleTrait, and we dispatch on it for things like npoint (https://juliageo.org/GeoInterface.jl/stable/guides/defaults/#Fallbacks). Besides, as an interface, it doesn't decide on how something should be represented in memory.

Quick example for a LineString:

julia> using GeoInterface
julia> l = GeoInterface.Wrappers.Line([[1,2], [1,2]])
GeoInterface.Wrappers.Line{false, false, Vector{Vector{Int64}}, Nothing, Nothing}([[1, 2], [1, 2]], nothing, nothing)

julia> @code_llvm GeoInterface.npoint(l)
;  @ /Users/evetion/.julia/packages/GeoInterface/8VGPL/src/interface.jl:146 within `npoint`
define i64 @julia_npoint_566({ {}* }* nocapture noundef nonnull readonly align 8 dereferenceable(8) %0) #0 {
top:
  ret i64 2
}

I would propose to remove the "like it's done in GeoInterface for example" from the sentence above. A replacement, if needed, could be like it's done in GIS/Simple Features representations (such as WKT/WKB, etc).

juliohm commented 11 months ago

You are taking a specific implementation of the interface, in this case a GeoInterface.Wrappers.Line and trying to prove a point about the interface, which is not valid. The GeoInterface.jl is still an interface with the issues raised in the book.