Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert GeointerfaceMakie into a weak dependency #99

Open
felixcremer opened this issue May 6, 2023 · 8 comments
Open

Convert GeointerfaceMakie into a weak dependency #99

felixcremer opened this issue May 6, 2023 · 8 comments

Comments

@felixcremer
Copy link
Member

Should GeointerfaceMakie be a weak dependency in Julia 1.9? This would enable the Makie recipes for all packages that implement the geointerface e.g. archgdal

@rafaqz
Copy link
Member

rafaqz commented May 6, 2023

Unfortunately all the package actually need to apply the macro to their own types because Makie cant dispatch on traits.

But we could make it a weak dependency everywhere, or even just a dependency as its only MakieCore.jl. But maybe weak is best.

Someone just has to make the PRs:

  • Shapefile.jl
  • ArchGDAL.jl
  • LibGEOS.jl
  • GeoJSON.jl

@rafaqz
Copy link
Member

rafaqz commented May 6, 2023

I've got a PR to update GeoInterfaceMakie.jl so vectors of geometries and Features/FeatureCollections also work. So these PRs should add the macro to geometries, features and feature collections at the same time where the packages have them.

Also may as well add Plots.jl recipes where they are missing at the same time!

@felixcremer
Copy link
Member Author

I tried to load GeoInterfaceMakie in a weak dependency for ArchGDAL which would be loaded as soon as Makie is loaded, but I ran into world age problems. I now added GeoInterfaceMakie as a plain dependency, because that seems to be easier.

@asinghvi17
Copy link
Member

asinghvi17 commented Feb 5, 2024

Could we also make it a weak dependency of GeoInterface, to get wrapper geometries enabled by default? It would just add GeoInterfaceMakieExt to GeoInterface in addition to the regular GeoInterfaceMakie. We could do the same for Plots as well.

@rafaqz
Copy link
Member

rafaqz commented Feb 5, 2024

Wont we get some circular dependencies?
GeoInterfaceMakie depends on GeoInterface

Makie also depends on GeoInterface.jl via GeometryBasics.jl

We should make that an extension

@asinghvi17
Copy link
Member

I don't think so - GeoInterfaceMakie will only ever load after GeoInterface and Makie are both loaded, i.e., after Makie is loaded (because that triggers the check for applicable extensions AFAIK).

We could make GeoInterface an extension to GB easily, though. It's just one file which doesn't define any exportable methods (we can move the definition of GB.geointerface_geomtype as an empty shell somewhere else in GB proper).

@rafaqz
Copy link
Member

rafaqz commented Feb 5, 2024

Ok, give it a try then

@asinghvi17
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants