search

JuliaPlots / PlotlyJS.jl

Julia library for plotting with plotly.js
Other
413 stars 77 forks source link

Windows error for PlotlyKaleido breaks precompilation #482

Closed disberd closed 4 months ago

disberd commented 4 months ago

This recent PlotlyKaleido PR https://github.com/JuliaPlots/PlotlyKaleido.jl/pull/17 introduced an error when the kaleido process hangs and does not accept inputs (this should only happen on Windows 10/11 when using Kaleido_jll v0.2).

This was done as on many windows machine (but apparently not all), the execution of the Kaleido_jll library (only for version > 0.1) hangs soon after start.

This is reflected in the diverse amount of related issues in the various kaleido repositories (e.g. https://github.com/JuliaPlots/PlotlyJS.jl/issues/473, https://github.com/JuliaPlots/PlotlyKaleido.jl/issues/13, https://github.com/plotly/Kaleido/issues/134, https://github.com/plotly/Kaleido/issues/110 and others) and is a problem of the kaleido C library that is used by all the various plotly packages to export figures.

This problem is only relevant to some windows machines, and the wide-spread suggested solution is to download to Kaleido (C library) 0.1 to avoid the problem.

The PR in https://github.com/JuliaPlots/PlotlyKaleido.jl/pull/17 caused the call to PlotlyKaleido.start() to actually throw an error if the underlying process does not appear to respond (which again should only happen on Windows and for Kaleido v0.2), as the kaleido library is anyhow useless in those situation and at least an error with a suggestion on a possible fix is (in my opinion, the author of the PR) is better than a forever hanging process.

Now, in a comment to the aforementioned PR, @montyvesselinov made me notice that this breaks precompilation of PlotlyJS even if he doesn't actually use Kaleido.

I see that a call to Kaleido.start() is happening in the __init__() function of PlotlyJS: https://github.com/JuliaPlots/PlotlyJS.jl/blob/75eacf4542dc12d7b05242b550f250e6039929bf/src/PlotlyJS.jl#L102-L105

This was added in #481 but was actually there from the start.

My question is now the following: Do we need to pre-start the kaleido library in the init of PlotlyJS to achieve maximum TTFX when calling savefig?

I see three possible ways to solve the precompilation error:

  1. Avoid calling PlotlyKaleido.start in the __init__ function, so that it's only actually started when someone actually calls savefig (this should already happen as the savefig code is already checking for the running process)
  2. Change to code in PlotlyKaleido.jl so that the error is not thrown during the PlotlyKaleido.start() call, but when the savefig function is called (similar to 1., but might speed up TTFX of savefig for PlotlyJS only in case the user is not on Windows with 0.2.1 of Kaleido).
  3. Change the code in PlotlyKaleido to avoid throwing an error (or avoid doing it by default), but this does not actually changes the fact that if the process hangs, Kaleido is not usable and so a different library version must be installed (i.e. 0.1) for it to be used

I expressed the possible solution that came to my mind in my order of preference. If 2 was to be implemented, for consistency I believe we should avoid throwing errors all-together in PlotlyKaleido.start as also the other currently present errors are thrown if the library is not actually usable.

@BeastyBlacksmith, do you have any comment on this and suggestions on proposed way-forward?

montyvesselinov commented 4 months ago

This needs to be fixed. I do not think it is ok PlotlyKaleido to break the compilation of PlotlyJS. In many cases, PlotlyKaleido is not used at all when PlotlyJS is executed.

This should be just a warning message.

BeastyBlacksmith commented 4 months ago

I do agree that PlotlyJS should remain usable even if the kaleido process hangs. I also think it should issue a warning at using time if that is the case to prevent surprises when trying to save.

disberd commented 4 months ago

This is the error message that is just logged with https://github.com/JuliaPlots/PlotlyKaleido.jl/pull/18

Precompilation doesn't fail but savefig will not work. It will actually error (after re-displaying the log error as it will try to restart the kaleido process)

image

montyvesselinov commented 4 months ago

For me it fails:

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.10.2 (2024-03-01)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> import PlotlyJS
ERROR: InitError: It looks like the kaleido process is hanging.
If you are on windows this might be caused by known problems with Kaleido v0.2 on windows.
You might want to try forcing a downgrade of the kaleido library to 0.1.
Check the Package Readme at https://github.com/JuliaPlots/PlotlyKaleido.jl/tree/main#windows-note for more details
Stacktrace:
  [1] error(s::String)
    @ Base .\error.jl:35
  [2] readline_noblock
    @ C:\Users\monty\.julia\packages\PlotlyKaleido\aujjS\src\PlotlyKaleido.jl:52
  [3] #start#6
    @ C:\Users\monty\.julia\packages\PlotlyKaleido\aujjS\src\PlotlyKaleido.jl:128
  [4] start
    @ C:\Users\monty\.julia\packages\PlotlyKaleido\aujjS\src\PlotlyKaleido.jl:59 [inlined]
  [5] __init__()
    @ PlotlyJS C:\Users\monty\.julia\packages\PlotlyJS\b6MbQ\src\PlotlyJS.jl:104
  [6] run_module_init(mod::Module, i::Int64)
    @ Base .\loading.jl:1134
  [7] register_restored_modules(sv::Core.SimpleVector, pkg::Base.PkgId, path::String)
    @ Base .\loading.jl:1122
  [8] _include_from_serialized(pkg::Base.PkgId, path::String, ocachepath::String, depmods::Vector{Any})
    @ Base .\loading.jl:1067
  [9] _require_search_from_serialized(pkg::Base.PkgId, sourcepath::String, build_id::UInt128)
    @ Base .\loading.jl:1581
 [10] _require(pkg::Base.PkgId, env::String)
    @ Base .\loading.jl:1938
 [11] __require_prelocked(uuidkey::Base.PkgId, env::String)
    @ Base .\loading.jl:1812
 [12] #invoke_in_world#3
    @ .\essentials.jl:926 [inlined]
 [13] invoke_in_world
    @ .\essentials.jl:923 [inlined]
 [14] _require_prelocked(uuidkey::Base.PkgId, env::String)
    @ Base .\loading.jl:1803
 [15] macro expansion
    @ .\loading.jl:1790 [inlined]
 [16] macro expansion
    @ .\lock.jl:267 [inlined]
 [17] __require(into::Module, mod::Symbol)
    @ Base .\loading.jl:1753
 [18] #invoke_in_world#3
    @ .\essentials.jl:926 [inlined]
 [19] invoke_in_world
    @ .\essentials.jl:923 [inlined]
 [20] require(into::Module, mod::Symbol)
    @ Base .\loading.jl:1746
during initialization of module PlotlyJS

julia> @isdefined PlotlyJS
false
disberd commented 4 months ago

I was just showing an example putting in an environment PlotlyJS and the dev branch of the PR.

The fix on PlotlyKaleido is not merged yet. I guess you were just trying with latest PlotlyJS with the regular published version of PlotlyKaleido?

montyvesselinov commented 4 months ago

Thank you! I will wait! However, I still think it is better to label this as WARNING rather than ERROR. But I might be wrong.

disberd commented 4 months ago

@montyvesselinov try again and make sure that while adding PlotlyJS version 2.2.4 of PlotlyKaleido (the latest) is installed.

Now you should only get a warning during using or precompilation and get an error only when trying to call savefig

image

montyvesselinov commented 4 months ago

Excellent! I will test it soon. Thank you so much!

https://envitrace.com/

Velimir “monty” Vesselinov

Co-Founder & CTO - Envitrace

+1 505-473-4150

@.***> https://calendly.com/montyv https://github.com/montyvesselinov https://www.linkedin.com/in/montyvesselinov https://montyv.github.io/

On Tue, Mar 5, 2024 at 10:26 AM Alberto Mengali @.***> wrote:

@montyvesselinov https://github.com/montyvesselinov try again and make sure that while adding PlotlyJS version 2.2.4 of PlotlyKaleido (the latest) is installed.

Now you should only get a warning during using or precompilation and get an error only when trying to call savefig

image.png (view on web) https://github.com/JuliaPlots/PlotlyJS.jl/assets/12846528/475a275f-952e-4a80-98a0-8bafb5371461

— Reply to this email directly, view it on GitHub https://github.com/JuliaPlots/PlotlyJS.jl/issues/482#issuecomment-1979277873, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABK7C6B3TIK6Z76K5VAFLN3YWX533AVCNFSM6AAAAABEFICHOGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZZGI3TOOBXGM . You are receiving this because you were mentioned.Message ID: @.***>