JuliaPlots / PlotlyJS.jl

Julia library for plotting with plotly.js
Other
422 stars 78 forks source link

Remove Blink dependency? #184

Closed EricForgy closed 6 years ago

EricForgy commented 6 years ago

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

@require HTTP include("HTTP.jl")

@require HttpServer include("HttpServer.jl")

Would you be open to a PR that does something similar:

@require Blink include("Blink.jl")

where PlotlyJS.Blink includes PlotlyJS methods specific to Blink that only get loaded if a user does something like:

> using Blink
> using PlotlyJS

The using Blink would need to appear before using 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.

sglyon commented 6 years ago

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.

aviks commented 6 years ago

That sounds useful. Would PlotlyBase.jl work with Plots.jl?

sglyon commented 6 years ago

Yes, in principle it could, but you wouldn't gain much (if anything) over the plotly backend for Plots.jl

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 PlotlyWebIO.jl. This is an experimental and un-registered package where I am testing out the ability for WebIO to replace all my custom/handwritten display integration and logic in PLotlyJS.jl. If it works, the current display code will be stripped out of PlotlyJS.jl and replaced by a WebIO based solution. But, that should be a non-breaking change and the user should never know or worry about the usage of WebIO or my current code

EricForgy commented 6 years ago

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 eval (and is already available with callbacks using Pages.jl - and probably WebIO.jl (note-to-self: Look at WebIO 😅 )).

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 to Pages.jl so now you can just use

Plotly.newPlot(graphdiv,data,layout,options)

in your Julia code just like the JS API and it renders to a web page, where data, layout are from PlotlyBase.jl and options is something you can expect to see in a PlotlyBase.jl PR soon (if it isn't there already) 😊