Breaking: remove geointerface #260
Conversation
|
It can't be an extension here? |
|
Then we can't have an extension on MakieCore or GeometryBasics in GeoInterface, as it's still a loop |
|
Ah yeah, that was the problem we ran into when I tried that the first time...this is probably a better plan then. You will need to keep a stub |
|
Maybe? I was thinking to shift |
|
Maybe have a call to design how the plotting extensions should look like before making breaking changes? I'm having a hard time going through the dependencies and what counts as circular. If we keep this here, can we solve it with extensions elsewhere? Like |
|
Yeah, there may be other approaches too, but we need to somehow get the Makie extensions sorted out. We can compare this approach to @asinghvi17 PR that made GeoInterface an extension. I'll try to make a matching GeoInterface PR so there is something concrete to discuss, I find it hard to talk about this without something concrete. This was just the first and easiest PR, and can be used in other PRs as a dependency so they can pass CI. |
|
I converted it to a draft to make it clear the GeoInterface code needs to be ready first, should have done that from the start |
|
So this is the error you get at the moment if you add a Makie extension to GeoInterface: ulia> using Makie
┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│ DimensionalDataMakie [71847b91-7361-5cb1-9c72-7bdc55e4d1c2]
│ GridLayoutBase [3955a311-db13-416c-9275-1d80ed98e5e9]
│ ArchGDALMakieExt [ed506d0f-917e-5ac5-847b-cce2fb23f093]
│ RastersNCDatasetsExt [bba50282-38eb-5d15-bdf0-02a8d9bd9f97]
│ Shapefile [8e980c4a-a4fe-5da2-b3a7-4b4b0353a2f4]
│ RastersArchGDALExt [003ceca8-0bef-59e8-b8ec-a536193683ee]
│ GeoInterfaceMakieExt [9330a4a3-9f70-5e4e-8a3b-c2816733222c]
│ ArchGDAL [c9ce4bd3-c3d5-55b8-8973-c0e20141b8c3]
│ Tyler [e170d443-b9d6-418a-9ee8-061e966341ef]
│ GeometryOpsDataFramesExt [da3db1d4-308d-5ad7-8cec-66cc688e85da]
│ ShaderAbstractions [65257c39-d410-5151-9873-9b3e5be5013e]
│ GeoInterfaceMakie [0edc0954-3250-4c18-859d-ec71c1660c08]
│ RastersMakieExt [dcb859c5-a41b-597c-97ab-c4a11b2128cf]
│ ShapefileMakieExt [ce22dd63-8704-5a0b-b9df-feab1218054b]
│ GeometryOps [3251bfac-6a57-4b6d-aa61-ac1fef2975ab]
│ GeometryOpsProjExt [414de2eb-6a2d-54ce-92ca-a96eb037817d]
│ GeoDataFrames [62cb38b5-d8d2-4862-a48e-6a340996859f]
│ Proj [c94c279d-25a6-4763-9509-64d165bea63e]
│ MathTeXEngine [0a4f8689-d25c-4efe-a92b-7142dfc1aa53]
│ MeshIO [7269a6da-0436-5bbc-96c2-40638cbb6118]
│ SortTileRecursiveTree [746ee33f-1797-42c2-866d-db2fce69d14d]
│ GLMakie [e9467ef8-e4e7-5192-8a1a-b1aee30e663a]
│ Makie [ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a]
│ MapTiles [fea52e5a-b371-463b-85f5-81770daa2737]
│ Packing [19eb6ba3-879d-56ad-ad62-d5c202156566]
│ RastersStatsBaseExt [0bd19630-9938-51cf-9b6f-d8cb9aa5cba5]
│ GeometryOpsCore [05efe853-fabf-41c8-927e-7063c8b9f013]
│ GeoInterfaceRecipes [0329782f-3d07-4b52-b9f6-d3137cf03c7a]
│ GeometryBasics [5c1252a2-5f33-56bf-86c9-59e7332b4326]
│ MakieCore [20f20a25-4f0e-4fdf-b5d1-57303727442b]
│ OldConScape [3ba2d6f2-f651-5162-bda8-601e45ce5244]
│ Rasters [a3a2b9e3-a471-40c9-b274-f788e487c689]
│ GeometryBasicsExt [b238bd29-021f-5edc-8b0e-16b9cda5f63a]
│ RastersProjExt [d192b857-01f0-5017-b366-ea56bcca1608]
│ PointClouds [3cc211ed-1f43-4d33-a894-629186066e97]
│ FreeTypeAbstraction [663a7486-cb36-511b-a19d-713bb74d65c9]
│ RastersCoordinateTransformationsExt [1dcd6f10-9ec3-58bb-b0b4-fd0e5104d14b]
│ ConScape [2ba2d6f2-f651-5162-bda8-601e45ce5244]
└ @ Pkg.API ~/.julia/juliaup/julia-1.10.5+0.x64.linux.gnu/share/julia/stdlib/v1.10/Pkg/src/API.jl:1279
[ Info: Precompiling Makie [ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a]
┌ Warning: Module Makie with build ID ffffffff-ffff-ffff-0000-1ae8e0f401dd is missing from the cache.
│ This may mean Makie [ee78f7c6-11fb-53f2-987a-cfe4a2b5a57a] does not support precompilation but is imported by a module that does.
└ @ Base loading.jl:1948
┌ Error: Error during loading of extension GeoInterfaceMakieExt of GeoInterface, use `Base.retry_load_extensions()` to retry.
│ exception =
│ 1-element ExceptionStack:
│ Declaring __precompile__(false) is not allowed in files that are being precompiled.
│ Stacktrace:
│ [1] _require(pkg::Base.PkgId, env::Nothing)
│ @ Base ./loading.jl:1999
│ [2] __require_prelocked(uuidkey::Base.PkgId, env::Nothing)
│ @ Base ./loading.jl:1812
│ [3] #invoke_in_world#3
│ @ ./essentials.jl:926 [inlined]
│ [4] invoke_in_world
│ @ ./essentials.jl:923 [inlined]
│ [5] _require_prelocked
│ @ ./loading.jl:1803 [inlined]
│ [6] _require_prelocked
│ @ ./loading.jl:1802 [inlined]
│ [7] run_extension_callbacks(extid::Base.ExtensionId)
│ @ Base ./loading.jl:1295
│ [8] run_extension_callbacks(pkgid::Base.PkgId)
│ @ Base ./loading.jl:1330
│ [9] run_package_callbacks(modkey::Base.PkgId)
│ @ Base ./loading.jl:1164
│ [10] _tryrequire_from_serialized(modkey::Base.PkgId, path::String, ocachepath::String, sourcepath::String, depmods::Vector{Any})
│ @ Base ./loading.jl:1487
│ [11] _require_search_from_serialized(pkg::Base.PkgId, sourcepath::String, build_id::UInt128)
│ @ Base ./loading.jl:1574
│ [12] _require(pkg::Base.PkgId, env::String)
│ @ Base ./loading.jl:1938
│ [13] __require_prelocked(uuidkey::Base.PkgId, env::String)
│ @ Base ./loading.jl:1812
│ [14] #invoke_in_world#3
│ @ ./essentials.jl:926 [inlined]
│ [15] invoke_in_world
│ @ ./essentials.jl:923 [inlined]
│ [16] _require_prelocked(uuidkey::Base.PkgId, env::String)
│ @ Base ./loading.jl:1803
│ [17] macro expansion
│ @ ./loading.jl:1790 [inlined]
│ [18] macro expansion
│ @ ./lock.jl:267 [inlined]
│ [19] __require(into::Module, mod::Symbol)
│ @ Base ./loading.jl:1753
│ [20] #invoke_in_world#3
│ @ ./essentials.jl:926 [inlined]
│ [21] invoke_in_world
│ @ ./essentials.jl:923 [inlined]
│ [22] require(into::Module, mod::Symbol)
│ @ Base ./loading.jl:1746
│ [23] include
│ @ ./Base.jl:495 [inlined]
│ [24] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vec
tor{Pair{Base.PkgId, UInt128}}, source::Nothing)
│ @ Base ./loading.jl:2222
│ [25] top-level scope
│ @ stdin:3
│ [26] eval
│ @ ./boot.jl:385 [inlined]
│ [27] include_string(mapexpr::typeof(identity), mod::Module, code::String, filename::String)
│ @ Base ./loading.jl:2076
│ [28] include_string
│ @ ./loading.jl:2086 [inlined]
│ [29] exec_options(opts::Base.JLOptions)
│ @ Base ./client.jl:316
│ [30] _start()
│ @ Base ./client.jl:552
└ @ Base loading.jl:1301
┌ Warning: attempting to remove probably stale pidfile
│ path = "/home/raf/.julia/compiled/v1.10/MakieCore/ALu4Q_2wleI.ji.pidfile"
└ @ FileWatching.Pidfile ~/.julia/juliaup/julia-1.10.5+0.x64.linux.gnu/share/julia/stdlib/v1.10/FileWatching/src/pidfile.jl:244 |
|
Looks like on 1.11 moving this code to an extension with anshuls old PR #212 is enough to allow adding a Makie extension in GeoInterface.jl. So this PR may not be needed. We will have to check 1.10 as it previously had more problems with extensions where the extending package was already loaded by another dependency. Then the question is do we need both - GeometryBasics will be a weak dep in GeoInterface for the Makie extension anyway. |
This PR can be rolled with the next breaking release.
The reason is to break the circular dependencies between GeoInterface and GeometryBasics extensions in regards to plot recipes.
We will move all of this code to an extension in GeoInterface.jl. GeoInterfaceMakie.jl limits GeometryBasics.jl to 0.5 in most setups. There is a (very) small chance to get both GeoInterface and GeometryBasics 0.6 at the same time (if basically no other JuliaGeo deps are installed), which may break some code. So we will need to have the GeoInterface version with GeometryBasics.jl 0.6 ready to tag straight away whenever this is tagged.
@evetion @SimonDanisch @asinghvi17