JuliaGeo / GeoInterface.jl

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

Design how *blessed* implementation of GeoInterface would work as generic fallback. #141

Open evetion opened 2 months ago

evetion commented 2 months ago

Or how we could use GeometryOps as a default implementation of methods such GeoInterface.area, for packages that do define a geometry, but not the method.

If GeometryOps now would implement GeoInterface.area(::geomtrait, geom), it would be type piracy.

evetion commented 2 months ago

Random thought:

We choose a single blessed package, and implement the "type piracy" method as a package extension here at GeoInterface.

So something like GeometryOpsExt.jl will have area(::geomtrait, geom::Any) = GeometryOps.area(::geomtrait, convert(:GeometryOps, geom))?

Then again, single blessed packages are restrictive (and no single one might've fully implemented the spec) so we could also go with the above package extension route, but add dispatch on the module (possible in 1.10, will be LTS soon):

area(::geomtrait, geom::Any) = area(::Val{backend}, ::geomtrait, convert(backend, geom)), and we introduce a get_backend() method that users can set/call, or is set by the package extension (last or first package loaded comes first?).

edit: The Val{backend} way also enables implementations of several packages directly on our Wrapper geometries, without introducing type piracy, so we don't need to do the convert.

evetion commented 2 months ago

Might be useful for inspiration on what to do, or probably, what not to do from Plots: https://github.com/JuliaPlots/Plots.jl/blob/master/src/backends.jl

asinghvi17 commented 2 months ago

The package extension here would be a circular dependency, since any such package would have depend on GeoInterface. This is the same issue with GeoInterfaceMakie and GeoInterface geoms.

asinghvi17 commented 2 months ago

set by the package extension (last or first package loaded comes first?)

seems pretty fragile - what if a dependency loads one of the packages that sets a backend?

Maybe a set priority list would be better but I would rather centralize all of this in GeometryOps, at least until we understand how people use the design. It already has a LibGEOS backend for example, we could simply add more (GDAL, S2) as required.

evetion commented 2 months ago

The package extension here would be a circular dependency, since any such package would have depend on GeoInterface. This is the same issue with GeoInterfaceMakie and GeoInterface geoms.

Hmm, I keep forgetting about that. :/

Well then, packages could implement area(::Val{package}, ::geomtrait, geom::Any/Wrapper), and we have a method here that you can set? Either user facing like Plots with setting gr() or pyplot(), or something in the blessed packages?

rafaqz commented 2 months ago

I think we should instead deprecate GeoInterface methods like area.

Using LibGEOS and GeometryOps methods directly is simpler and works on all objects without confusion over what package is doing what.

But I think we should standardise the function form and arguments in the implementing packages.

evetion commented 1 month ago

But I think we should standardise the function form and arguments in the implementing packages.

That seems like a no-regret action to me, like #134 for read/write.

However, ideally we would somehow have a (correct for crs trait) default fallback that just works, just like Plots works out of the box. But it might be too hard to do correctly 🤷🏻 ?

rafaqz commented 1 month ago

read/write is a good example! It's basically the same problem.

So we're talking about an "interface" where package specific functions have an identical definition following a specification, rather than methods extending a single function.

That's an interesting kind of interface. For ecosystem level user experience consistency rather than anything technical.

We could test it with a test function that accepts the function as an argument and calls it on generic GeoInterface geoms and keywords.

evetion commented 1 month ago

Ok, after our nice little talk and some reflection, let's deprecate the operations here 💥.

Motivation; accessing geometries is fundamentally different from operations on geometries. Different type of geometries often come from file drivers (99% of our implementations), and have no operations. Operations are hard and scarce.

Impact of doing this is probably minimal, and for the packages implementing area and such we should still implement GeoInterface.area(::Any) and related interface methods as fallback to not "break" the contract. But we do not document it, and only document GO for operations.

In terms of operations we keep the package/module trait (area(LibGEOS(), stuff), so we can compare our own implementations, users can choose which implementation they want, and in rare cases other developers can extent these operations and make another package extension.

We might want to reconsider LibGEOS() and go for Val{LibGEOS} (as in the module) if that produces a more elegant design.