Skip to content
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

Plotly.react #2

Closed
rreusser opened this issue Jul 31, 2017 · 28 comments
Closed

Plotly.react #2

rreusser opened this issue Jul 31, 2017 · 28 comments

Comments

@rreusser
Copy link
Contributor

Seeking feedback from @etpinard @alexcjohnson @bpostlethwaite on the best location for Plotly.react. I can imagine a few possibilities:

  1. it should go in plotly.js and we just have to add a couple lines of code to this repo to make use of it.
  • Pro: react-like functionality usable from any library
  • Con: more code in plotly.js
  1. it goes in this repo. It computes a diff (maybe using the schema to figure out what to do about array data, with optional flag to apply immutable-like reference-change logic to simplify diff computation) and sends the result to the relevant plotly.js API methods.
  • Pro/con: opposites of above

Unless I'm wrong, sounds like @alexcjohnson is/will be working on Plotly.react. I just wanted to confirm that's what this will link up with.

@bpostlethwaite
Copy link
Member

My vote is for this repo. Once we agree on that lets get it imported into https://github.com/plotly/Plotlyjs-React-Lab

@rreusser
Copy link
Contributor Author

@bpostlethwaite Can you clarify what you mean by imported?

@bpostlethwaite
Copy link
Member

well I wrote a Plotly React component for the Lab. https://github.com/plotly/Plotlyjs-React-Lab/blob/master/src/components/PlotlyReact.js

The intention is to replace it with the finished version that will be hosted elsewhere.

@bpostlethwaite
Copy link
Member

which, if here, should be imported into the lab so we can use it within that test application.

@alexcjohnson
Copy link
Collaborator

My initial thought was to put it into the plotly.js repo so it would work from anyone's home-grown plotly react component - or even not in a react environment at all, which I guess is @rreusser 's Pro above. If we put it in here, what would it take for someone to use it with plotly.js outside react, or at least outside of our react component?

@bpostlethwaite I would imagine that the lab wouldn't actually need to call Plotly.react directly at all itself... this component would swap it in for Plotly.newPlot, the lab would include this component, and that would be all you'd need, right?

@rreusser
Copy link
Contributor Author

We could certainly expose it separately, as in require('plotlyjs-react/diff') or something.

@rreusser
Copy link
Contributor Author

@bpostlethwaite I assumed you meant the component would be imported. Is that correct? (As opposed to a standalone method that computes a diff and used in a react component)

@bpostlethwaite
Copy link
Member

Oh sorry for the confusion. Yes I was thinking component it makes lots of sense that it is in Plotly.js

@rreusser
Copy link
Contributor Author

rreusser commented Jul 31, 2017

Tricky parts of Plotly.react:

  • detecting changes in data without change in array reference. Like if you modify a data_array entry, how does the component know to deep-diff and locate the change? The ugly method is to specify immutable vs. expensive style where you must explicitly request that it check for value changes without reference changes.
  • two-way binding. How should it handle UI zooming? So you:
    1. zoom in the UI
    2. axis range values are modified in the original object
    3. those values are passed back into the component
    4. it doesn't detect a by-value change since they've already been modified? That could work, but it feels kinda sketchy.

@chriddyp
Copy link
Member

How should it handle UI zooming? So you:

I'm not sure if this is where folks are discussing requirements or not, but a few more things from my (Dash's) end:

Currently, calling newPlot resets the zoom and legend items. Some users will want zoom and legend items to reset, other users will not (https://community.plot.ly/t/preserve-trace-visibility-in-callback/5118), and some users will want to give that choice up to the user (exposing e.g. "Lock Camera" control)

image

@rreusser
Copy link
Contributor Author

@chriddyp Yeah, I think the goal is a method that more or less diffs the input and sends it to the correct method (relayout, resytyle, update, animate, etc). newPlot would be the naive fallback. That's why the general format of including the component is:

var Plotly = require('plotly.js')
var createPlotlyComponent = require('plotlyjs-react')
var PlotlyComponent = createPlotlyComponent(Plotly)

(to be just a bit verbose.) The important part is that you can inject your own version (including cdn if you prefer) and if it has Plotly.react, your component will be smart and optimized, if not, then it'll newPlot everything.

@alexcjohnson
Copy link
Collaborator

Currently, calling newPlot resets the zoom and legend items.

That's exactly the issue - that changes to the state driven by the plot's own UI do not propagate back to the state you used to generate the plot in the first place. How do we handle this? I'm not sure it's feasible to say "these are the pieces of the state that can get modified by the UI, so ignore these in the input to newPlot and keep what was set on the plot" - which if I'm understanding it right is what you're thinking of with things like "lock camera" and freeze=True - and anyway it feels to me like it would be much more powerful if we could push those changes back into the app state, and use that to flow back through the plot via Plotly.react.

@chriddyp in general I feel like the user's intent is already clear from their UI actions - if they zoom in, then autorange gets turned off for those axes and data updates will not re-autorange; but if they haven't zoomed in, autorange will be true still so a data update will give new ranges. Certainly you could imagine particular scenarios where you want to override that, but as long as these state changes get pushed back up to the app level I think the default behavior is pretty good as a default.

So how do we accomplish this? Maybe all the internal Plotly.relayout/restyle/update calls should be wrapped by something that knows whether the plot is in a react environment, and if so is able to redirect to an action that modifies the state, which eventually then trickles back to the plot via Plotly.react? Or perhaps the easiest is to have this component listen to all events that might accompany state modifications (I guess plotly_restyle, plotly_relayout, and plotly_update) and expose a handler (onPlotChange?) that allows the dev to connect this back into their app state? That way normally the plot would already have updated itself and when the change flows back down to the plot, it will see no diff, but if the app wanted to respond to UI-driven changes with further edits - say, you zoom in and it refines the data it's displaying - it would be able to simply make that change like any other app action.

Relatedly, @rreusser points out that it's not very react-like, nor good for performant diffing, for plotly.js to be mutating data and layout. @etpinard I'd like your thoughts on this but we've had users get tripped up by gd.data and gd.layout mostly being mutated but sometimes being replaced, would it be too big a change for us to never mutate these, only replace them?

@rreusser
Copy link
Contributor Author

rreusser commented Aug 2, 2017

@alexcjohnson good points. I think the cleanest is for the component to connect to the events and trigger a callback. It could even expose a plotly_changed event that emits the changed properties when you interact via the UI.

With this general approach the plot wouldn't have to know it's in a react environment, right?

@rreusser
Copy link
Contributor Author

Okay, to summarize some of the conclusions from various discussions, in particular with @alexcjohnson, here's the basic design regarding react:

  1. When you create a react component, it will clone the data and layout passed in. (There's the possibility of not cloning data arrays; I haven't fully decided on that matter). That data will be stored on the react component instance and passed to Plotly.plot.
  2. When you interact with the plot, it will modify its internal copy of the data. It will be 100% impossible for plot interactions to mutate the data passed to the component.
  3. When a plot interaction occurs, a plotly_react event will be emitted. The plotly component will subscribe to this and receive a set of changes applied to gd.data/layout. It will in turn issue those changes via a onChange callback passed to the plotly react component.
  4. The user will apply those changes to their copy of the data via a helper method that lives on the plotly api, i.e. Plotly.applyChanges.
  5. The user then passes that data back to the plotly component, which checks the updated data and layout against its copy, sees there have been no changes, and does not re-render.
  6. If new data or layout are passed to the component, the component will issue Plotly.react from its componentDidUpdate method, which will perform the updates.
  7. A revision counter prop will optionally trigger a full replot so that you have some control over forcefully refreshing. This is also the fallback for forcing an update only when you want one when not using Plotly.react.

@nicolaskruchten
Copy link
Contributor

I think I followed this correctly, and it's likely that react-pivottable will avail itself of point 7 and force a replot every time the UI changes, at least for now :)

@rreusser
Copy link
Contributor Author

rreusser commented Oct 18, 2017

@nicolaskruchten Sounds good! Alternatively, maybe it's worthwhile to add a "don't be clever" switch that tells it to replot even when Plotly.react is available (which would amount to a not-even-one-line override when checking the availability of Plotly.react).

@nicolaskruchten
Copy link
Contributor

Yeah the no_cleverness flag might be a nice feature for me :)

@Nauss
Copy link

Nauss commented Oct 20, 2017

I'm using plotly.js in a React environment and I would like to add this:

  • For 1., we have large datasets to display (in tens of millions of points) so, for us it would not be possible to "copy all the data". Iterators would work.
  • We need to resize the plot (its containing div) quite often and this operation must be efficient.

This is just a quick remark, I'll be happy to go into more details if needed.

@rreusser
Copy link
Contributor Author

rreusser commented Oct 20, 2017

@Nauss Thanks for the feedback!

Regarding your first point, that's the precise concern. The standard react-like approach is to clone everything, compare it to the new input, and apply changes. For plotly.js, that won't work for cases like yours. We've been talking about applying some heuristics instead. Specifically:

  • Copy only data array references to the internal data store. Plotly has a schema so that we know whether particular entries in the input are data or non-data attributes.
  • When the plot receives new input, compute a deep diff for everything except the data arrays.
  • If a data array reference or length has changed, then we obviously need to update the data.
  • If a data array reference has not changed, then it's a bit trickier. The best options seem:
    • an immutable mode that only checks the reference (and length, perhaps)
    • a non-immutable mode that computes a deep diff of arrays (which would take perhaps 1ms for >90% of plots)
    • a revision counter so that you can tell it when the plot has been revised and it needs, at the very least, to redraw the traces. (implemented currently, though it leads to a full newPlot instead of just indicating it needs to restyle the traces.)

As for resizing, there are perhaps a couple options. First, I added a fit prop that lets you automatically adjust the plot to the size of the parent container when the window resizes. This is a common use case, but not very general. For more general resizes, you should be able to just change the size in the layout and it will decide it only needs to execute Plotly.relayout to resize the plot—which is the best performing way to resize a plot.

Hope that addresses your concerns! Please feel free to follow up though!

@Nauss
Copy link

Nauss commented Oct 20, 2017

Thanks for the quick answer :)

  • About the data, it looks good to me.
  • About the resizing, it is actually a direct user interaction (not a window resizing). I would like to achieve smooth "mouse resizing" (I'm using react-rnd for the resize). It would not be a problem if the Plotly.relayout is done only at the end (i.e. mouse up).

We have a naive implementation for now but I'll try v1.0 next and come back here for a feedback.

Thanks for the great work !!

@rreusser
Copy link
Contributor Author

@Nauss Sounds good! Just to be clear, v1.0 does do a full replot on every change. It's meeting our current need, though it might not be adequate for you until v2.0. Regarding the mouse resizing, I think there's no reason you couldn't use overflow: hidden on your panel and not pass the updated size layout until mouseup.

@swiperii
Copy link

swiperii commented Dec 17, 2017

We are trying to use react-plotly.js in our react app, which displays e.g. scatter plots of real-time production data. That is, our data series are updated with new data regularly. And without a Plotly.react method, all user interactions (zoom area, point selections, toolbar choice,...) are of course reset for each new data point.

Is there any way to avoid this with the current plotly react component? Any workaround that would allow us to add data without completely resetting the plot state?

If not, what is the current status of the Plotly.react method? Any chance that this type of scenario will be supported in the near future?

bpostlethwaite added a commit to plotly/react-chart-editor that referenced this issue Jan 6, 2018
This is temporary fix until
plotly/react-plotly.js#2
has been released
bpostlethwaite added a commit to plotly/react-chart-editor that referenced this issue Jan 6, 2018
This is temporary fix until
plotly/react-plotly.js#2
has been released
bpostlethwaite added a commit to plotly/react-chart-editor that referenced this issue Jan 6, 2018
This is temporary fix until
plotly/react-plotly.js#2
has been released
bpostlethwaite added a commit to plotly/react-chart-editor that referenced this issue Jan 6, 2018
This is temporary fix until
plotly/react-plotly.js#2
has been released
@nicolaskruchten
Copy link
Contributor

@swiperii if you want to create a separate issue I'd be happy to try to help you accomplish your goals within the current framework. Most likely you'll need to handle the updates and feed the layout back to the props along with the new data points.

@nicolaskruchten
Copy link
Contributor

To clarify the next steps... The Plotly.react functionality will be added to plotly.js, and progress on that can be tracked here: plotly/plotly.js#1850

Once the upstream functionality is in place, this React component will be modified to take advantage of it and we can then close this current issue :)

@swiperii
Copy link

@nicolaskruchten Thanks for your response. I have currently made my own Plotly.react method that uses add/deleteTraces, restyle and some ugly hacks to make data updates work together with zoom and selections. There are still some bugs though, so I eagerly look forward to the real thing :)

@nicolaskruchten
Copy link
Contributor

Good news! The Plotly.react method is implemented in plotly.js version 1.34.0 and this React wrapper already takes advantage of it, so just upgrade plotly.js and things should be more reactive!

@AyoCodess
Copy link

is there a solution for this?. As the plot automatically re-draws when any the data object changes. i.e an x-axis data point.

@bpostlethwaite
Copy link
Member

There is a Plotly react component that was produced from this here: https://github.com/plotly/react-plotly.js/

There is also the .react method on the vanilla plotly object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants