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#redraw (1.20.4.min.js) appears to not clean up properly #1218

Closed
czellweg opened this issue Dec 2, 2016 · 12 comments
Closed

Plotly#redraw (1.20.4.min.js) appears to not clean up properly #1218

czellweg opened this issue Dec 2, 2016 · 12 comments
Labels
bug something broken

Comments

@czellweg
Copy link

czellweg commented Dec 2, 2016

I have the following, simple code (also see attached zip for standalone:

<!doctype html>
<html>
<head>
  <meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
  <title>plot.redraw multiple axes</title>
  <script src="plotly-1.20.4.min.js"></script>
</head>
  <body>
    <div id="myDiv1"></div>
    <div id="myDiv2"></div>
    <script>
      var trace1 = {
        x: [1, 2, 3],
        y: [40, 50, 60],
        name: "yaxis data",
        type: "scatter"
      };
      var trace2 = {
        x: [2, 3, 4],
        y: [4, 5, 6],
        name: "yaxis2 data",
        yaxis: "y2",
        type: "scatter"
      };
      var layout1 = {
        title: "Remove yaxis, use redraw()",
        yaxis: {title: "yaxis title"},
        yaxis2: {
          title: "yaxis2 title",
          titlefont: {color: "rgb(148, 103, 189)"},
          tickfont: {color: "rgb(148, 103, 189)"},
          overlaying: "y",
          side: "right"
        }
      };
      var data = [trace1, trace2];
      Plotly.newPlot("myDiv1", data, layout1);
      Plotly.newPlot("myDiv2", data, layout1);

      setTimeout(function() {
        delete layout1.yaxis2;
        delete trace2.yaxis;
        var plot = document.querySelector("#myDiv1");
        // plot.layout = layout1;
        // redraw appears to not clean out the old data
        Plotly.redraw("myDiv1");

        var layout2 = {
          title: "Using Plotly#newPlot seems to work",
          yaxis: {title: "yaxis title"}
        };
        // this works as expected
        Plotly.newPlot("myDiv2", data, layout2);

      }, 2*1000);

    </script>

  </body>
</html>

What I get is the following:
screen shot 2016-12-02 at 14 26 34

As you can see, the upper plot is being updated with Plotly#redraw while the lower plot is updated using Plotly#newPlot.
I would expect those two invocations to produce the same result. However, in the one using Plotly#redraw, it appears as thought the previous 2nd y-axis and the associated trace is not cleaned up properly. You can also see that when the mouse is hovered over the trace, the 'old' trace is not (correctly so) taken into account, i.e. no hover-text appears over the 'old' trace.

I have verified this behaviour on a Mac OS 10.10.5 and the following browsers:

  • Chromium, Version 55.0.2876.0 (64-bit)
  • Chrome, Version 54.0.2840.98 (64-bit)
  • Safari, Version 9.1.2 (10601.7.7)
  • Firefox, Version 50.0.2

plotly-multiple-axes.html.zip

@rreusser
Copy link
Contributor

rreusser commented Dec 2, 2016

In live example form: http://codepen.io/rsreusser/pen/xRpeYX?editors=0010

@rreusser
Copy link
Contributor

rreusser commented Dec 2, 2016

Here's a slight modification in which I pulled in clone-deep from wzrd.in. Need to think about exactly why it's happening, but it seems maybe the problem is related to passing the same trace to two different plots. Appears they mutate it.

http://codepen.io/rsreusser/pen/ObzGZG?editors=1010

(Thanks for reporting!)

@czellweg
Copy link
Author

czellweg commented Dec 2, 2016

@rreusser That was fast! ;)
Okay, deep-cloning data might not be a solution for us as we're having a lot of data points in some cases. I will have a look though whether having separate traces does the trick.

@rreusser
Copy link
Contributor

rreusser commented Dec 2, 2016

Yeah, definitely seems like a reasonable workaround. (Also, of course only one of two plots actually needs a clone.) I think there was a bit of discussion related to this, so I'll check around and get @etpinard's input before digging further at the moment.

@etpinard etpinard added the bug something broken label Dec 2, 2016
@czellweg
Copy link
Author

Has there been any internal discussion so far on this issue @rreusser ? I'm using the latest version (1.24.2) and it appears to still be an issue.

@rreusser
Copy link
Contributor

rreusser commented Mar 15, 2017

For reference: the clearest illustration of the result that I can see is below. The first plot gets a phantom trace that's not anywhere in its data. I think it's the result of the second trace dumping its SVG output into the first trace because it looks up something by id and gets the wrong plot.

1218

Digging a bit deeper: there just aren't that many properties on the top level data and layout objects. layout.*axis.range has a long history of getting mutated by view updates, but that wouldn't cause this. In fact, from what I can tell, uid is really the only property that's mutated on the input objects themselves. If there's only one common object used by both plots, then 1. one plot sets the uid on the data object, and 2. the second plot (depending on whether plot commands are side by side or chained asynchronously) either uses the same uid or overrides the uid with a different value. Later on, something is looking up an element by an id attribute that's computed from the uid. It gets the wrong element and SVG output ends up on the wrong plot.

Long story short: duplicate id attributes on DOM elements resulting from conflicting uids is my best guess.

My conservative suggestion was a deep clone which may be unnecessary. Perhaps a shallow clone is enough to fix the uid issue, though the axis range might still have problems with just a shallow clone of the input. It's probably the best that can be done for now.

cc @etpinard @alexcjohnson for second opinion

@alexcjohnson
Copy link
Collaborator

It doesn't look to me like an issue with the data and layout objects. I think we're just not properly exiting elements from the svg when a subplot disappears like this. Note that we do a bit better with stacked subplots instead of overlaid:
screen shot 2017-03-15 at 2 08 16 pm
But even there the old axis title isn't cleaned up.

The easy fix, I think, would be to have Plotly.redraw delete all children of the plotDiv before its call to Plotly.plot. That may be fine, as we intend to phase out redraw anyway, though of course the longer-term solution should be to refactor the plot framework routines to be more d3-idiomatic.

@rreusser
Copy link
Contributor

Gah, sorry. I thought it was cross-polluting the data, but I think I misinterpreted the input/output.

@fhurta
Copy link
Contributor

fhurta commented Jun 5, 2017

I wanted to report a new bug but probably it is the same bug reported here?

In the app the traces are added and removed, but after few operations, there remain some gridlines.

Observed with version 1.27.1. See this code pen

@czellweg
Copy link
Author

czellweg commented Nov 2, 2017

Has there been any progress with this issue? Or is there a roadmap / idea when this can be expected to be resolved?

@etpinard
Copy link
Contributor

etpinard commented Nov 2, 2017

@czellweg redraw will soon be deprecated and be replaced by #1850 - so don't expect this ticket to be resolved anything soon.

In the meantime, I suggest rewriting your logic using Plotly.restyle, Plotly.relayout or Plotly.update.

@alexcjohnson
Copy link
Collaborator

This seems to have been solved, partially by #2227 and partially by something before it.

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

No branches or pull requests

5 participants