JuliaMath / Polynomials.jl

Polynomial manipulations in Julia
http://juliamath.github.io/Polynomials.jl/
Other
303 stars 75 forks source link

MakieCore doesn't need to be in `deps` #575

Closed bluesmoon closed 3 months ago

bluesmoon commented 3 months ago

I see MakieCore is in deps as well as weakdeps in the Project.toml file. Since it's only used by the PolynomialsMakieCoreExt extension, I don't believe it needs to be in deps as well. Can it be removed as it's pulling in a large number of dependencies.

jverzani commented 3 months ago

Thanks!

bluesmoon commented 3 months ago

Thanks. I see it implemented in #576. Does a new version get triggered directly?

jverzani commented 3 months ago

Thanks again. I really appreciate your pointing this out.

fhagemann commented 3 months ago

This causes the following error in Julia 1.6...

ERROR: LoadError: LoadError: ArgumentError: Package Polynomials does not have MakieCore in its dependencies:
- If you have Polynomials checked out for development and have
  added MakieCore as a dependency but haven't updated your primary
  environment's manifest file, try `Pkg.resolve()`.
- Otherwise you may need to report an issue with Polynomials
Stacktrace:
  [1] require(into::Module, mod::Symbol)
    @ Base ./loading.jl:906
  [2] include(mod::Module, _path::String)
    @ Base ./Base.jl:384
  [3] include(x::String)
    @ Polynomials ~/.julia/packages/Polynomials/aeqze/src/Polynomials.jl:6
  [4] top-level scope
    @ ~/.julia/packages/Polynomials/aeqze/src/Polynomials.jl:64
  [5] include
    @ ./Base.jl:384 [inlined]
  [6] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt64}}, source::String)
    @ Base ./loading.jl:1235
  [7] top-level scope
    @ none:1
  [8] eval
    @ ./boot.jl:360 [inlined]
  [9] eval(x::Expr)
    @ Base.MainInclude ./client.jl:446
 [10] top-level scope
    @ none:1
in expression starting at /home/runner/.julia/packages/Polynomials/aeqze/ext/PolynomialsMakieCoreExt.jl:1
in expression starting at /home/runner/.julia/packages/Polynomials/aeqze/src/Polynomials.jl:1

Could you use Requires to load the extension code only when MakieCore is explicitly loaded?


@static if !isdefined(Base, :get_extension)
    using Requires
end

function __init__()
    @static if !isdefined(Base, :get_extension)
        @require MakieCore = "20f20a25-4f0e-4fdf-b5d1-57303727442b" include("../ext/PolynomialsMakieCoreExt.jl")
    end
end
jverzani commented 3 months ago

Thanks. (Now I know why I had that in there.) I'll see if your suggestion works, as I'd really love to drop MakieCore from deps/

fhagemann commented 3 months ago

Heres some guideline on how to do backwards compatibility with extensions: https://pkgdocs.julialang.org/dev/creating-packages/#Backwards-compatibility

bluesmoon commented 3 months ago

@jverzani your jverzani:issue_575 branch works well for me on Julia 1.6.7

jverzani commented 3 months ago

Yes, now to get it to work without Aqua errors. Hopefully it will sort out today.

fhagemann commented 3 months ago
Aqua.test_all(Polynomials; stale_deps=(;ignore=[:Requires]))
jverzani commented 3 months ago

Merci!!

fhagemann commented 3 months ago

No problem, thanks for taking care of this so quickly :)

bluesmoon commented 3 months ago

Fixed in 4.0.11