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

Clean up constraint groups #2465

Merged
merged 1 commit into from
Mar 9, 2018
Merged

Clean up constraint groups #2465

merged 1 commit into from
Mar 9, 2018

Conversation

alexcjohnson
Copy link
Collaborator

Fixes #2462

The problem was that _axisConstraintGroups gets reset in cartesian-only code, so with no cartesian module, the old one just gets added back in by relinkPrivateKeys. So I cleaned that up, but also realized that without cartesian we don't even need to do some of the steps in Plotly.plot (including the one that errored) 🐎 In fact either one of those would have been enough to fix this bug, but I figured both can't hurt.

@etpinard we should really rethink relinkPrivateKeys - can we perhaps figure out which keys we really need to relink, and give them some stronger marker than just the generic underscore, which also means private, not an attribute? There are probably other keys that can cause similar problems...

@etpinard
Copy link
Contributor

etpinard commented Mar 9, 2018

Looks good! 💃

we should really rethink relinkPrivateKeys - can we perhaps figure out which keys we really need to relink,

In #749, I thought about making each module (trace, subplot and component) list which keys need to be relink on redraws. At that time, it sounds like a bigger hassle that it was worth. But now, with almost 2-years worth of additional tests, we could maybe try to swap the generic loop-over-all-keys relink step with something more fine-grained?

@alexcjohnson alexcjohnson merged commit c0a97bd into master Mar 9, 2018
@alexcjohnson alexcjohnson deleted the drop-constraints branch March 9, 2018 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants