search

JuliaPlots / Plots.jl

Powerful convenience for Julia visualizations and data analysis
https://docs.juliaplots.org
Other
1.84k stars 354 forks source link

[BUG] Fails to precompile Plots.jl #3964

Closed ValentinKaisermayer closed 2 years ago

ValentinKaisermayer commented 2 years ago 
julia> import Plots
[ Info: Precompiling Plots [91a5bcdd-55d7-5caf-9e0b-520d859cae80]
ERROR: LoadError: LoadError: too many parameters for type
Stacktrace:
  [1] _precompile_()
    @ Plots ***\.julia\packages\Plots\AGxtZ\deps\SnoopCompile\precompile\precompile_Plots.jl:92
  [2] top-level scope
    @ ***\.julia\packages\Plots\AGxtZ\src\precompile_includer.jl:14
  [3] include(mod::Module, _path::String)
    @ Base .\Base.jl:384
  [4] include(x::String)
    @ Plots ***\.julia\packages\Plots\AGxtZ\src\Plots.jl:1
  [5] top-level scope
    @ ***\.julia\packages\Plots\AGxtZ\src\Plots.jl:267
  [6] include
    @ .\Base.jl:384 [inlined]
  [7] 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::Nothing)
    @ Base .\loading.jl:1235
  [8] top-level scope
    @ none:1
  [9] eval
    @ .\boot.jl:360 [inlined]
 [10] eval(x::Expr)
    @ Base.MainInclude .\client.jl:446
 [11] top-level scope
    @ none:1
in expression starting at ***\.julia\packages\Plots\AGxtZ\src\precompile_includer.jl:9
in expression starting at ***\.julia\packages\Plots\AGxtZ\src\Plots.jl:1
ERROR: Failed to precompile Plots [91a5bcdd-55d7-5caf-9e0b-520d859cae80] to ***\.julia\compiled\v1.6\Plots\jl_9DF5.tmp.
Stacktrace:
  [1] error(s::String)
    @ Base .\error.jl:33
  [2] compilecache(pkg::Base.PkgId, path::String, internal_stderr::Base.TTY, internal_stdout::Base.TTY, ignore_loaded_modules::Bool)
    @ Base .\loading.jl:1385
  [3] compilecache(pkg::Base.PkgId, path::String)
    @ Base .\loading.jl:1329
  [4] _require(pkg::Base.PkgId)
    @ Base .\loading.jl:1043
  [5] require(uuidkey::Base.PkgId)
    @ Base .\loading.jl:936
  [6] require(into::Module, mod::Symbol)
    @ Base .\loading.jl:923
  [7] eval
    @ .\boot.jl:360 [inlined]
  [8] eval
    @ .\Base.jl:39 [inlined]
  [9] repleval(m::Module, code::Expr, #unused#::String)
    @ VSCodeServer ***\.vscode\extensions\julialang.language-julia-1.5.6\scripts\packages\VSCodeServer\src\repl.jl:157
 [10] (::VSCodeServer.var"#69#71"{Module, Expr, REPL.LineEditREPL, REPL.LineEdit.Prompt})()
    @ VSCodeServer ***\.vscode\extensions\julialang.language-julia-1.5.6\scripts\packages\VSCodeServer\src\repl.jl:123
 [11] with_logstate(f::Function, logstate::Any)
    @ Base.CoreLogging .\logging.jl:491
 [12] with_logger
    @ .\logging.jl:603 [inlined]
 [13] (::VSCodeServer.var"#68#70"{Module, Expr, REPL.LineEditREPL, REPL.LineEdit.Prompt})()
    @ VSCodeServer ***\.vscode\extensions\julialang.language-julia-1.5.6\scripts\packages\VSCodeServer\src\repl.jl:124
 [14] #invokelatest#2
    @ .\essentials.jl:708 [inlined]
 [15] invokelatest(::Any)
    @ Base .\essentials.jl:706
 [16] macro expansion
    @***\.vscode\extensions\julialang.language-julia-1.5.6\scripts\packages\VSCodeServer\src\eval.jl:34 [inlined]
 [17] (::VSCodeServer.var"#53#54")()
    @ VSCodeServer .\task.jl:411

Versions

(test) pkg> st
      Status `***\test\Project.toml`
  [91a5bcdd] Plots v1.24.4

julia> versioninfo()
Julia Version 1.6.4
Commit 35f0c911f4 (2021-11-19 03:54 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i5-6200U CPU @ 2.30GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, skylake)
Environment:
  JULIA_PKG_PRECOMPILE_AUTO = 0
  JULIA_EDITOR = code
  JULIA_NUM_THREADS = 4
ValentinKaisermayer commented 2 years ago

This is the line:

Base.precompile(Tuple{Core.kwftype(typeof(gr_polyline)),NamedTuple{(:arrowside, :arrowstyle), Tuple{Symbol, Symbol}},typeof(gr_polyline),StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64},Vector{Float64}})
ValentinKaisermayer commented 2 years ago

Has been changed in https://github.com/JuliaPlots/Plots.jl/compare/v1.24.3...v1.24.4#diff-b4bb7ce0302d8c1bbac1f5e78646fde19a8e230367e81a3c7eb7f5b4e2dc0aafR92

BeastyBlacksmith commented 2 years ago

I can't reproduce that on Ubuntu 20.04 julia v1.7.0, but I can on julia v1.6.2

juliohm commented 2 years ago

Same issue here. Whoever released this version without waiting for tests to pass on Julia v1.6 is irresponsible.

Perhaps this bug is not seen in GitHub Actions in the test suite? Can you please explain how Plots.jl still produces releases with such serious bugs? It is kind of alarming.

simonbyrne commented 2 years ago

Can this be yanked?

BeastyBlacksmith commented 2 years ago

Whoever released this version without waiting for tests to pass on Julia v1.6 is irresponsible.

Tests where only run on julia v1.7.0 (where they passed), since the CI script is not updated yet to test also on v1.6.

simonbyrne commented 2 years ago

Release 1.24.4 has now been yanked from registry: https://github.com/JuliaRegistries/General/pull/49708 Either the compat bounds need to be updated, or the precompile statements need to be versioned.

KristofferC commented 2 years ago

Whoever released this version without waiting for tests to pass on Julia v1.6 is irresponsible. Perhaps this bug is not seen in GitHub Actions in the test suite? Can you please explain how Plots.jl still produces releases with such serious bugs? It is kind of alarming.

https://github.com/JuliaPlots/Plots.jl/blob/b1c56126fb777f9f3b09251f745b51565bb627cd/.github/workflows/ci.yml#L25 got automatically changed to refer to 1.7 when it was released and thereby 1.6 was no longer tested. You seem to have the same problem in your packages, e.g. https://github.com/JuliaEarth/GeoStats.jl/blob/eae0a0a1f6b32c0c28bfccf9dc1a61328c04f7d3/.github/workflows/CI.yml#L20 while still declaring compat for julia 1.5.

I wouldn't be too quick to talk about irresponsibility...

t-bltg commented 2 years ago

@KristofferC, do you advise us to use explicit minor numbers in all .ymls (1.7 instead of 1) ?

simonbyrne commented 2 years ago

You should at least have the minor version of the lowest minor version you support.

juliohm commented 2 years ago

@KristofferC I love how you always try to use my own packages to prove a point even when the order of magnitude is hugely different. You want to compare the user base of Plots.jl with that of GeoStats.jl? Is that the comparison you want to make really?

GeoStats.jl is still in version v0.x so users aren't expecting stability. Plots.jl is in version v1.24 so yes I expect much more responsibility in releases. Don´t compare apples to oranges, please.

simonbyrne commented 2 years ago

We all make mistakes, but statements like "Whoever released this version without waiting for tests to pass on Julia v1.6 is irresponsible" are completely unhelpful. It wouldn't have even helped in this case, since the tests were set to run on the latest released version (which is now 1.7).

juliohm commented 2 years ago

The problem is that these issues aren't rare. They are so frequent that they became the norm in Plots.jl It is a huge project with a huge user base and almost every user of Julia will encounter these issues upfront.

I reported a couple of issues like these last year where simply loading the package or trying to use a basic function causes an error. This shouldn't be the norm. I need to share these concerns energetically otherwise they will keep happening with the current standards.

simonbyrne commented 2 years ago

The problem is that these issues aren't rare. They are so frequent that they became the norm in Plots.jl It is a huge project with a huge user base and almost every user of Julia will encounter these issues upfront.

I reported a couple of issues like these last year where simply loading the package or trying to use a basic function causes an error. This shouldn't be the norm. I need to share these concerns energetically otherwise they will keep happening with the current standards.

And it's thankless work to maintain such a massive package. Comments like the above make it even harder.

juliohm commented 2 years ago

Sometimes hard is necessary.

I maintain dozens of Julia packages as well thanklessly, but I try to do my best to catch basic issues. Why? Because I know people depend on some of these packages to do their work. There is no excuse. They count on me (the maintainer of the package) and so I try to spend extra time (with the limited resources I have) to check everything before a new release.

If Plots.jl breaks and students have issues, I don't think it fine to just say "hey, this is open source, they are allowed to release a plotting package that breaks often because no one is paying them, it is thankless work". So coming back to my point: I don't think the situation in Plots.jl is inevitable. It is happening because of poor software engineering practices and probably because casual contributors gained power to make releases, which is completely not fine. Making a release is a huge responsibility, even bigger when it comes to a package like Plots.jl that is used by most users of the language.

BeastyBlacksmith commented 2 years ago

You are welcome to improve the standards of Plots.jl, but I don't think ranting is helping anything with that regard.

juliohm commented 2 years ago

I would love to help. I simply can't find the time. I am already taking care of a lot of packages across multiple Julia organizations and don't have extra energy to maintain Plots.jl. The release of GeoStats.jl alone requires the release of ~20 packages within JuliaEarth plus a couple of other packages in JuliaML and JuliaGeometry.

The Plots.jl project needs an official maintainer who is responsible for making reviews before any release. If that position is not filled, it should be filled by someone that contributes actively to the project and knows it well.

simonbyrne commented 2 years ago

The Plots.jl project needs an official maintainer who is responsible for making reviews before any release. If that position is not filled, it should be filled by someone that contributes actively to the project and knows it well.

Again, unless you're volunteering time or money, this is not particularly helpful.

juliohm commented 2 years ago

It is helpful in the sense that active contributors to Plots.jl can read this message and volunteer to become the ones responsible for reviewing releases. Alternatively it is helpful because even if no one is willing to take the position it shows a path to a solution which could be simply some form of standardization in the release process with a set of well defined steps that any volunteer can help check. Either way the message highlights the issue and is therefore helpful.

On Wed, Dec 1, 2021, 19:44 Simon Byrne @.***> wrote:

The Plots.jl project needs an official maintainer who is responsible for making reviews before any release. If that position is not filled, it should be filled by someone that contributes actively to the project and knows it well.

Again, unless you're volunteering time or money, this is not particularly helpful.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JuliaPlots/Plots.jl/issues/3964#issuecomment-984126112, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3L3GWJOEWYSBXTISXDUO2QNRANCNFSM5JE5JWOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

KristofferC commented 2 years ago

I love how you always try to use my own packages to prove a point even when the order of magnitude is hugely different. You want to compare the user base of Plots.jl with that of GeoStats.jl? Is that the comparison you want to make really?

GeoStats.jl is still in version v0.x so users aren't expecting stability. Plots.jl is in version v1.24 so yes I expect much more responsibility in releases. Don´t compare apples to oranges, please.

Please. You don't see how silly it looks when you complain like this while doing exactly the same with your own package (that you announce a bunch on discourse and even have GSoC proposals for)? Pot, meet kettle.

Your question was "Can you please explain how Plots.jl still produces releases with such serious bugs?". The answer is that the exact same mistake you are doing occurred here (a mismatch between the CI matrix and the julia compat range) due to changes to what 1 refers to in the Julia GitHub actions testing script.

So know both you and the people here are aware of it and we can focus on reducing these types of mistakes in the future. Perhaps the registration bot should have some check that you run CI on the earliest Julia version you claim to be compatible with for example.