-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Plotly.animate doesn't unset attributes #1533
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
Comments
Can you clarify the precise result you see or expect to see? It seems to me like it's working: Here's a minimal codepen that queries it directly: http://codepen.io/rsreusser/pen/ZemVqQ?editors=0010 : Plotly.plot(gd,
[{y: [1, 2, 3], marker: {opacity: 0.5}}]
).then(function () {
console.log('Before animation: marker.opacity =', gd._fullData[0].marker.opacity)
return Plotly.animate(gd,
[{data: [{'marker.opacity': null}]}]
)
}).then(function () {
console.log('After animating to null: marker.opacity =', gd._fullData[0].marker.opacity)
}) In the console I see:
|
@rreusser I expected |
Thanks. I understand now. I guess I considered the behavior functionally equivalent, but I could be wrong about that. Just to help plan the best course of action, can you clarify whether this breaks things—as opposed to being an inconvenience which I agree should perhaps be changed? |
The relevant line is in Plots.extendObjectWithContainers#L1614: dest = Lib.extendDeepNoArrays(dest || {}, expandedObj); Since it's a deep extend, it does not automatically unset null properties. The challenge is that we would need |
It doesn't break anything for me at the moment. I've only noticed that |
@n-riesco @rreusser this situation is similar to #1410. I have no strong opinion here to be honest. I can think of situation where pruning make sense and others that don't. Better yet, maybe as @n-riesco suggested yesterday we should expose some Moreover, @n-riesco's point about |
Might be reiterating a point from that ticket, but to clarify and apart from validation, is there ever a case where // an option is not settable if it's either:
// - missing (so is irrelevant given other settings)
// - an array (so contains data that you don't want to override)
function notSettable(val, arrayOk) {
return val===undefined || (Array.isArray(val) && !arrayOk);
} (i.e. null is settable, right?) |
Is this really a big deal? Should we do anything about it? |
Quick update: almost all of the pruning logic got 🔪 in @alexcjohnson 's e84d4b9 |
Closing. This doesn't seem to cause any important problems. |
Here's a codepen that illustrates the issue.
I expected that:
would unset `gd.data[0].marker.opacity.
The text was updated successfully, but these errors were encountered: