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

Move property literals out of Plotly.restyle #648

Closed
monfera opened this issue Jun 15, 2016 · 5 comments
Closed

Move property literals out of Plotly.restyle #648

monfera opened this issue Jun 15, 2016 · 5 comments

Comments

@monfera
Copy link
Contributor

monfera commented Jun 15, 2016

See items 1, 2, 3 in #617 (comment).
Preceding comment from @etpinard: #617 (comment)

@etpinard
Copy link
Contributor

etpinard commented Jun 15, 2016

I think (at least I like to think) that there's a way to achieve this using our attribute declaration.

Some questions that we need to answer:

  • Do we need all restyle docalc, doplot, ... distinctions ? Can you merge some of them?
  • Most docalc updates relate to our data_array and our arrayOk attributes. Meaning that we could simply use the attribute declaration to figure our what update path to take. Are there any exceptions to that rule?
  • anything else ?

And most importantly, much of restyle (and relayout) is un-tested. We should be very careful when making this change.

@etpinard
Copy link
Contributor

From @rreusser 's #1578

Carpet is going to add a huge number of attrs to recalcAttrs. (up to 287 total or so!?) The array of attributes is recalculated on every plot and it looks them up with indexOf. It should place them in a hash (e.g. {mode: true, visible: true, ...}) and should be defined in a plot-api-global context since it never changes.

@alexcjohnson notes that a larger refactoring is desired, but this might be a start since, at least once carpet is merged, it shouldn't take more than five minutes and may very well be at the point of having a non-negligible impact on performance.

@etpinard
Copy link
Contributor

etpinard commented May 9, 2017

@etpinard
Copy link
Contributor

#1999 will probably close this.

@etpinard
Copy link
Contributor

etpinard commented Oct 5, 2017

Superseded by #2025

@etpinard etpinard closed this as completed Oct 5, 2017
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

2 participants