-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
Remove Blink dependency? #184
Comments
Hey @EricForgy thanks for the work you have been doing and for opening this issue up. I think that PlotlyJS.jl will probably always keep a dependency on Blink (or some electron window), but I believe you can achieve your goals by using PlotlyBase.jl instead. A couple months ago I had a use case where it was desirable for me to have the plot building functionality in a separate package from the one that handles display integrations and logic. So, I stripped those parts out and put them in the PlotlyBase.jl package. This does not depend on Blink. Do you think that this could work for you? If not, let me know and we can talk about how to make it work. |
That sounds useful. Would PlotlyBase.jl work with Plots.jl? |
Yes, in principle it could, but you wouldn't gain much (if anything) over the I'm a bit sorry for all the confusion regarding plotly and Julia. I'll try to explain how all the packages fit together: Plots.jl
My (registered) packages:
EDIT: There is also |
Hi @sglyon, Thanks for your note. I started having a look at both PlotlyJS.jl (after a long time) and PlotlyBase.jl. For my purposes, it does look like PlotlyBase.jl is sufficient 👍 I can see that Blink.jl is very ingrained into PlotlyJS.jl. In principle, that could be "fixed" and you could make the defaults go to a browser instead of Electron. That would be my preferred way since everyone has a browser that works and you just need a way to inject JS, which is easy to do with WebSockets and Most of the problems I have with PlotlyJS.jl are not PlotlyJS.jl related, but due to Blink.jl not building or something. Some of the other open issues indicate others experience the same. It would be unfortunate if problems with Blink.jl hold back the adoption of PlotlyJS.jl. I think PlotlyJS.jl could be a great default plotting tool for Julia newcomers, but if they bump into Blink.jl issues, that would be an understandable turn off. In any case, if Blink.jl (or other Electron-based packages) are going to continue to be a requirement for PlotlyJS.jl, then it likely means I will not be able to use it, which is a bummer, but I understand 😊 Btw, on the plane last night, I added Plotly.newPlot(graphdiv,data,layout,options) in your Julia code just like the JS API and it renders to a web page, where |
Hi @sglyon 👋
I was recently doing some work on WebSockets.jl. WebSockets.jl used to (past tense applies once the new branch is merged) depend on HttpServer.jl. I modified WebSockets.jl to be able to work with both HttpServer.jl and HTTP.jl, but in the process removed the dependency of WebSockets.jl on BOTH HttpServer.jl and HTTP.jl via Requires.jl.
WebSockets.jl now checks to see if you are using HttpServer.jl and / or HTTP.jl (it's cool they actual work nice together) and loads appropriate methods.
https://github.com/JuliaWeb/WebSockets.jl/blob/change_dependencies/src/WebSockets.jl#L390-L392
Would you be open to a PR that does something similar:
where
PlotlyJS.Blink
includes PlotlyJS methods specific to Blink that only get loaded if a user does something like:The
using Blink
would need to appear beforeusing PlotlyJS
.This has the advantage that PlotlyJS.jl no longer has Blink.jl as a dependency. If someone wants to use Blink.jl with PlotlyJS.jl, it is no longer your responsibility to worry about getting Blink installed and working.
This approach has grown on me. I like it for removing unnecessary package dependencies and would make your life easier I think.
What do you think?
Edit: I'm working on improving my team's workflow and I'd like to have PlotlyJS be a part of it, but I don't want to have Blink as a dependency for our workflow.
The text was updated successfully, but these errors were encountered: