Move Makie and Plots recipes to extensions

[rafaqz]

This PR removes the GeoInterfaceMakie and GeoInterfaceRecipes packages completely, and instead adds GeoInterfaceMakieExt and GeoInterfaceRecipesBaseExt extensions.

In this PR the macros to add recipes to geometries are:

GeoInterface.@enable_makie Makie MyGeometry
GeoInterface.@enable_plots RecipesBase MyGeometry

Theyre tested on the GeoInterface wrapper geometries, which means those will now just plot without loading an extra package.

The macro needs makie/plots in the name now as @enable could be anything outside of a plottting context.

It also needs the package as the first argument to avoid world age problems, so packages can just apply the macro from GeoInterface in their extensions on Makie/RecipesBase. It doesn't work moving the inner macro code to the extension where Makie/RecipesBase is available.

Closes #197
Closes #195
Closes #143

We should make a minor version bump after this, and all packages depending on GeoInterfaceMakie/GeoInterfaceRecipes will need to be switched to the macros defined here.

@SimonDanisch if you want to review, this should solve dependency problems.

[evetion]

This is great! Thanks for making the effort to revamp plotting. I will go through it later today. Small question regarding the world age, would invokelatest work for the enable macro?

[asinghvi17]

This looks great to me and should solve our problems. Thanks!

[rafaqz]

@evetion I'm not sure what would work, the problem is the macro has to define functions from packages that are not currently loaded in GeoInterface scope.

The macro calling a function in e.g. the Makie extension doesn't seem to work, it just calls the old stub function.

So passing it in was the easiest solution.

[asinghvi17]

GeometryBasics new version is now out.

[rafaqz]

Can we get a review and/or merge @evetion? I don't want to merge my own PR and get blamed for all the problems of the world again ;)

[evetion]

I've bashed my head against the world-age problem as well, and I couldn't find a quick way to do Base.@invoke_latest against a macro. I liked the approach I took in #143, defining the empty macro, just like a function, and extending it in the extension. That did seem to work for RecipesBase, but the situation with Makie and GeometryBasics is a bit more involved in terms of triggers.

I've tested this locally and it works great. It's a nice piece of work, and solves several issues in the ecosystem, kudos!

Supersedes #143, fixes #110.

[evetion]

You can use lts as well, although, like 1, that's a moving target.

[felixcremer]

Should we move the tests of the enable macro which lived in the GeoInterfaceRecipes to the GeoInterface tests?

[rafaqz]

I did! test_plots.jl and test_makie.jl

(The macros run on the GeoInterface geometry types now so we don't need tests of other objects)

[felixcremer]

Ah ok, sorry I misunderstood the codecov report and thought that the macros are not tested. But the drop in coverage is more because we don't test for mulitpoint and multilinestring which apperantly we did a bit more before. But this is not stopper for me and can be dealt with later. Also we are now also incorporating the coverage of the extensions which have not been shown before because they have been in subpackages.

[rafaqz]

Yeah, Makie broke MultiPoint

[SimonDanisch]

What would be a good test for multipoint support?
Just

scatter(MultiPoint(rand(Point2f, 10))
lines(MultiPoint(rand(Point2f, 10))

?
I guess it should just be treated like if called without MultiPoint wrapped around?

[rafaqz]

Really just scatter as lines would be undefined, the points are not known to be in any order, they're just discrete points that somehow form a collection.

But also Vectors of MultiPoint, so they can be e.g. colored by some shared value of each MultiPoint group.

scatter([MultiPoint(rand(Point2f, 10)), MultiPoint(rand(Point2f, 5))]; color=[:red, :green])

Although my color syntax might be wrong

[SimonDanisch]

The color grouping would definitely be a new feature... We could add a convert_arguments for the MultiPoints first, and later try to add the grouping... We do want a more generic way to expand attributes per_x anyways for other plots.

[rafaqz]

But doesn't LineString or Polygon do that already if you plot a vector of them?

[SimonDanisch]

Not that I know of...
Edit: poly([polys...]) does it, and it's implemented in the poly recipe.

[asinghvi17]

That kind of splitting doesn't natively exist yet but we should implement that. It will also resolve some long standing bugs with geomakie which is a plus.

Yeah poly has that only (iirc) because everything gets decomposed to a vector of meshes, and each multipoly also becomes a single mesh. If you tried lines for that syntax it should error.

[rafaqz]

Ok, I guess I assumed there was a general abstraction of that poly pattern for any vector of geometries. So yeah, that's my feature request!

Geospatial geometries are all expected to be able to represent the spatial component of some feature/table row, which has other associated values you often want to display visually. One point, a multipoint collection, a polygon, a multipolygon, a linestring etc, are all similar kinds of thing. So from a GeoInterface perspective we should aim for them all to behave identically, besides the detail of the actual geometric shape they make.

So usually we have a vector of many if these geometries to plot, and vectors of associated data, all in a table. Users will want to combine the geometry column with one or more of the data columns.

[rafaqz]

This is good to merge, please hit the button