-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
Get the plotly.min.js file from PlotlyJS.jl #4862
base: master
Are you sure you want to change the base?
Conversation
Can you please add the other PR here too, and let's see the combined PR runs. Plotly needs some fixing and maybe this would fix the failing tests on windows, otherwise looks very good. And perhaps we'd need to wait for the PR you made in PlotlyJs.jl too? |
Hey! The easiest would be to wait for JuliaPlots/PlotlyJS.jl#480 to be released, I can combine the PRs afterwards |
using REPL | ||
import PlotlyJS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@isentropic It looks like this caused a test failure because PlotlyJS is not a dependency. How does that work? I thought PlotlyJS is always loaded by default?
When I run import Plots; plotly(); plot(1:10)
in Pluto, does that load the PlotlyJS package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I run
import Plots; plotly(); plot(1:10)
in Pluto, does that load the PlotlyJS package?
No, only import Plots; plotlyjs(); plot(1:10)
will load PlotlyJS, plotly()
is special in that it is the only backend that is usable without any 3rd party (julia) package, but it can only display to a web browser, while plotlyjs
has a standalone display via Blink.jl
JuliaPlots/PlotlyJS.jl#480 is merged and released, but see my comment above |
probably plots gets loaded somewer here Line 561 in fcf2e74
|
not sure I never understood why plotly has more than 1 package |
Aha! Then I think this PR only makes sense if PlotlyJS was also added as a regular dep of Plots.jl, to ensure that it is always loaded and available to have access to the artifact. But maybe #4884 is a better alternative? |
There is code in Plots.jl to download the
plotly.min.js
file from the plotly CDN and store it in a scratch space, but the dependency PlotlyJS.jl already contains theplotly.min.js
as one of its artifacts! This PR uses this file for the plotly backend.This artifacts approach is very reliable (also on limited network connections, since artifacts are served by Julia's pkg server), so this PR reuses the file from PlotlyJS.jl in Plots.jl :)
Depends on JuliaPlots/PlotlyJS.jl#480