-
-
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
Clean up design #19
Comments
One idea I had was to make the current api methods (
I think we could then suggest that users use the |
Ok, so I'll try to outline my thought process here. I think what would help a lot to start with is to draw a clear distinction between plots and views of plots, as @spencerlyon2 mentioned above. A A In many cases, it might be convenient to synchronise views with their parent plot. I think the cleanest way to do this is to be completely explicit about it, and introduce a new type which joins the above two types together; in its simplest form: type SyncPlot
plot::Plot
view::PlotView
end
SyncPlot(p::Plot) = SyncPlot(p, display(p))
plot(...) = SyncPlot(Plot(...))
addtraces!(p::SyncPlot, ...) = (addtraces!(p.plot, ...); addtraces!(p.view, ...)) I think this relieves the tension between Does that seem sane to everyone? Would be great to hear what people think, whether this is the right path in general and/or there are refinements to be made. |
I think I understand your reasoning @one-more-minute, but from a practical perspective what does this framework allow that you couldn't easily do before? Is it primarily to enable multiple optional views? I thought Julia's multimedia io (displays, etc) already did that part. Why abstract on top of that?
When would you want to manipulate the "plot object" differently than the "view object"? I would think it makes more sense to implement |
Assuming the alternative is something similar to the current design -- where the plot and its views are treated as being part of the same mutable object -- there are a few reasons to be wary.
None of these is a real deal breaker on its own, all of them could probably be worked around, and the experience for a user at the repl probably isn't going to change that much. But for people building stuff on top of PlotlyJS.jl a really solid internal design would be a big boon. |
Hey @MikeInnes and @tbreloff I've implemented the If you have a chance to take a look at that PR and give any comments that'd be great -- no pressure of course. I'll close this in favor of the other one. |
This package is a bit tricky to design because we need to have both Julia and Plotly versions of a plot object.
The original goal was to have the Julia object be first class and simply have the plotly version be a "clone" of the Julia version that handles the view. I think that design was compromised, especially in the plotly.js api methods.
This issue is a place to brainstorm possible ways to clean up the design.
cc: @one-more-minute @tbreloff
The text was updated successfully, but these errors were encountered: