Skip to content

Plotly.deleteTraces leads to bad trace uid sequence #2836

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

Closed
etpinard opened this issue Jul 23, 2018 · 3 comments
Closed

Plotly.deleteTraces leads to bad trace uid sequence #2836

etpinard opened this issue Jul 23, 2018 · 3 comments
Labels
bug something broken regression this used to work

Comments

@etpinard
Copy link
Contributor

Consider

Plotly.newPlot('graph', [{
  y: [1, 2, 1]
}, {
  y: [2, 1, 2]
}, {
  y: [3, 2, 3]
}])
.then(gd => {
  console.log(gd._fullData.map(t => t.uid))
  // => ['0', '1', '2']
  return gd
})
.then(gd => {
  return Plotly.deleteTraces(gd, [0])
})
.then(gd => {
  console.log(gd._fullData.map(t => t.uid))
  // (currently) => ['0', '1']
  // (in 1.38.x) => ['1', '2']
})

In a codepen: https://codepen.io/etpinard/pen/KBmVYB?editors=1010

This is probably due to #2681 and probably leads to bugs (not yet reported) when deleting traces in subplots that track traces using uids e.g. gl3d:

for(i = 0; i < sceneData.length; ++i) {
data = sceneData[i];
if(data.visible !== true) {
continue;
}
trace = this.traces[data.uid];
if(trace) {
if(trace.data.type === data.type) {
trace.update(data);
} else {
trace.dispose();
trace = data._module.plot(this, data);
this.traces[data.uid] = trace;
}
} else {
trace = data._module.plot(this, data);
this.traces[data.uid] = trace;
}
trace.name = data.name;
}

cc @alexcjohnson

@etpinard etpinard added the bug something broken label Jul 23, 2018
@alexcjohnson
Copy link
Collaborator

Ah interesting... good catch, because of how getTraceUids pulls uids out of oldFullData, any operation that can alter existing trace indices - so not just delete but also move and add (if not adding at the end) - will have the same issue. I guess the easiest may be to mirror these operations in fullData? Are there similar bugs due to relinkPrivateKeys after a delete etc?

@etpinard etpinard added the regression this used to work label Jul 24, 2018
@etpinard
Copy link
Contributor Author

I guess the easiest may be to mirror these operations in fullData

This won't be trivial for graphs with one-to-many trace transforms (e.g. groupby), what if instead we mirror these operations on a uid stash?

@gvwilson
Copy link
Contributor

Hi - this issue has been sitting for a while, so as part of our effort to tidy up our public repositories I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our stack. Cheers - @gvwilson

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

No branches or pull requests

3 participants