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.js theme / style extraction and application #2469

Closed
bpostlethwaite opened this issue Mar 12, 2018 · 23 comments
Closed

plotly.js theme / style extraction and application #2469

bpostlethwaite opened this issue Mar 12, 2018 · 23 comments
Labels
feature something new

Comments

@bpostlethwaite
Copy link
Member

bpostlethwaite commented Mar 12, 2018

initial requirements

  1. Generic style only. Items that apply to any data setup such as trace color, size, background, and fonts are applied.
  2. The template data shall be stored without any design point data. For example, the x and y source data shall not be included. If an attribute such as color, size, or opacity is bound to a variable (has individual values), then the individual values will not be stored.
  3. The template shall be applied to an existing graph (a new graph is not generated).
  4. A mechanism shall be enabled through the API to report items from the template that do not apply. Example 1) The template has a different number of traces than the plot to which it is being applied. Example 2) The template is from a line plot and being applied to a heat map, therefore the point glyph cannot be applied.

cc @alexcjohnson @etpinard for implementation discussions
cc @chriddyp @sglyon for possible applications to and requirements of plotly.[py|jl] and Dash
cc @nicolaskruchten @VeraZab for general input.

To get the conversation rolling how about I take a stab at an API:

const figureStyle = plotly.extractStyle(gd || {data, layout}, config)
const info = plotly.applyStyle(gd || {data, layout}, {figureStyle [, config]} [, config])

where info is an object containing a subset of the structure in figureTheme documenting each item that was not applied and possibly an optional reason why it was not applied.

note: It would be great if this operation was immutable however point 3. requires mutation. Perhaps someone has suggestions for an API that allows both immutable application and mutation.

@bpostlethwaite
Copy link
Member Author

bpostlethwaite commented Mar 13, 2018

Okay how about:

const figureStyle = plotly.extractStyle(gd || {data, layout}, config)

// applyStyle does not mutate the data/layout you pass in or update the plot. 
// Simply returns a cloned and updated {data, layout} containing styling information and 
// an info object. Useful for using Plotly.js to build themed templates rather than operate
// directly on a plot. The config can control whether the data arrays are cloned or
// referenced.
const {info, figureWithStyle} = plotly.applyStyle({data, layout}, {figureStyle [, config]} [, config])

// mutates the {data, layout} you pass in and then calls plotlyjs.react to update the plot.
const {info, figureWithStyle} = plotly.updateStyle(gd, {figureStyle [, config]} [, config])

@alexcjohnson
Copy link
Collaborator

Plotly.extractStyle looks great - I'm assuming config is potential config for the extraction operation itself, as opposed to the plot config as in newPlot? I might change the name to extractTemplate or makeTemplate, which would accommodate future expansion beyond style information.

I don't think mutation itself (of data and layout) is a requirement, just the ability to update an existing plot (replacing gd.data and gd.layout)... so if we use:

// or Plotly.applyTemplate
const {info, figureWithStyle} = Plotly.applyStyle(gd || {data, layout}, {figureStyle [, config]} [, config])

then we can follow it up with:

Plotly.react(gd, figureWithStyle);

It's less compact to apply the style this way, but it feels to me like it may be preferable as it would give the dev a chance to look at info and decide if the templating worked as intended before going ahead and plugging the result into the plot.

The config can control whether the data arrays are cloned or referenced.

I would assume referenced is better and omit cloning until someone explicitly asks.

@bpostlethwaite
Copy link
Member Author

Yes the config is for configuring the Template extraction. I like the API you have provided and prefer makeTemplate over the word extract. Thanks!

@sglyon
Copy link

sglyon commented Mar 20, 2018

Hey @bpostlethwaite this looks awesome.

I only have a few points to add right now:

  • I think this is an improvement over the style/theme support Plotly.jl has right now -- it sounds really cool
  • In Julia I have the ability to merge themes. For example if I have one theme that just alters fonts and another that changes the default colors, I'd like to be able to easily combine them into a third theme that does both. I know we can apply one theme after another to the same plot, but being able to do it in one shot feels nice
  • Over in WIP: adding themes plotly.py#924 @jackparmer requested that themes be JSON serializable. I think this is a great idea

@nicolaskruchten
Copy link
Contributor

Generally +1, with a few comments:

  • I have a slight preference for "template" over "theme" and a dislike of "style"
  • the words "extract" and "apply" feel a bit like they imply mutation, so I would try to come up with something else like Plotly.buildTemplateFrom() and Plotly.buildFigureFromTemplate() or something?
  • do we distinguish consistently between "figure" referring to the JSON representation and "plot" being the actual plot in the DOM?

@bpostlethwaite
Copy link
Member Author

@nicolaskruchten sorry a more up-to-date current state is here: master...plotlyjs_templating

Your second and third points still apply though!

@alexcjohnson
Copy link
Collaborator

Had some slack discussions about this with @nicolaskruchten, particularly about how it will apply in the chart studio context. As well as the Initial requirements, we should be able to use this feature for theming there, which brings in a number of interrelated use cases:

  • Applying a template to an already-made plot (that's more or less what we're talking about here).
  • Setting a default template per user/group/company (we have a hacky per-company implementation here)
  • Applying a template and then later adding another trace, and expecting that trace to also conform to the template
  • (somewhat speculative) Retroactively applying template changes to existing plots that use this template - company changes its color scheme, or default font, etc.

Except for the first one, which covers the initial requirements above, all of these require that we hold on to the template as a property of the plot so that it can continue to be applied to new pieces (traces, axes, annotations, rangesliders, etc) as they are added, or to existing pieces as they are changed (scatter -> bar for example) rather than viewing templates simply as transformations that turn a figure into a new, templated, figure.

Now, if we have the template as part of the figure rather than a transformation TO the figure, we have the option of whether to apply it before the already-specified style attributes of the figure, or after (which is how it works here). I haven't played with them in enough detail to really know, but @nicolaskruchten says existing office suites (gdocs, ms office) seem to apply templates before user-modified attributes, ie if you've set a color or font or something explicitly and apply a template that has another value, you will keep your own value. My concern about doing it that way in the chart editor is that users don't have any way to know which settings are defaults and which have been explicitly set, or to un-set something that was previously set. There's also a question about which behavior users want or expect. I guess ideally we would give them both options, as we can come up with clear use cases for both, perhaps even on a per-attribute (or per-attribute-category) basis. If templates are transformations though, we really can't do both - either the template overwrites everything (ie applied after the user values, even resetting values not included in the template) or the next template application can't do anything because it doesn't know which settings are user-specified and which are from the previous template. But as a part of the figure, the normal mode would be to not override user values, and if you want to override you simply delete all the user values.

So what I'm thinking now is:

  • Template creation works as discussed and as @bpostlethwaite has begun implementing (though will need a little extension in case the figure used as the template base has a template of its own!)
  • Merging templates (@sglyon) is just an extendDeep since these are plain JSON 🎉
  • You apply a template to a figure by setting layout.template with this JSON
  • We implement THAT during supplyDefaults, by using the template value if there is no user value, and only if neither one exists do we fall back on the dynamic default (inherited or calculated based on other attributes) or the schema dflt. Note that because of dynamic defaults we can't just have the template directly override the schema dflt like we do in our existing template hack, it needs to be another step in the precedence chain.
  • We need a separate operation that strips user-supplied styles.
  • And @nicolaskruchten has assured me he will sort out indicating default/explicit and unsetting in the editor, and once we have ^^ in plotlyjs we can make some options in the template application GUI to clear some or all user styles.
  • For retroactive updates, we can use templatesrc, which will be the only dereference-able layout key, but it'll need special handling anyway since it's JSON not an array.

OK, that's a long comment... Any thoughts on all that?

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented May 15, 2018 via email

@etpinard
Copy link
Contributor

etpinard commented May 15, 2018

I can't think of any problems with @alexcjohnson's most recent comment. 👌

It would be nice though to get @chriddyp 's opinion on how this would fit with dash themes.

On the internal-side of things, I'm thinking it would be nice to write the themes logic as a component that we could keep out of src/core.js if the logic becomes too bloated 🌯

@alexcjohnson
Copy link
Collaborator

Question: what if someone wants a template to contain components in an object array? Like an annotation saying "Confidential," or the company logo in layout.images? We need a way for users to use, not use, or modify these without modifying the template itself.

@nicolaskruchten @VeraZab curious your thoughts on how this would fit with editor contexts and if you see alternatives.

A simple option, seems a bit awkward but does fulfill these needs:

  • If the new figure doesn't have that array at all, we just use the template array unchanged.
  • If the new figure does have that array, we generate an output array with the max of the two lengths, and combine objects pairwise.
  • If you want to omit one of the template array items, set your item to {visible: false} (currently it looks to me like all container arrays except updatemenus[i].buttons and rangeselector.buttons support visible - we should add it to those two anyway!). Anything else in these first items represents a modification to that template item.
  • If you want to create your own item in addition to the template items, first add {} for each template item. Creating these would be the responsibility of editor or client apps. I guess editors would need special logic to prevent deleting these objects too - you could only set them invisible.

That last part bothers me, having to insert some template-dependent number of {} before your own items. That makes changing templates awkward - any edits you make to these would be moot with the new template unless it contained substantially the same items in the same order, so what do you do in that case? Truncate this initial set if the new template has fewer items, and fill it with {} if it has more?

An alternative: append the template items to the end of the array, unless there's a specially-marked item, like {templateindex: 1, bgcolor: 'red'} which would pluck that element out of the template array, inserting it as modified at that point in the final array instead of appending it, and I guess if the template array doesn't have that item at all, turning it visible: false. Or perhaps even better, we could give the template items a name field and refer to them that way rather than by index, so that this modification would only apply to templates defining items of the correct name rather than all templates by index.

@nicolaskruchten
Copy link
Contributor

I like your 'alternative' at the end, personally!

I also have a related question/idea, which probably applies more to shapes and annotations... I would imagine that a template in this context would not be aimed at a specific annotation or shape but rather at being a template whereby ALL annotations and shapes would be styled. For example, "in company X, we use red lines with fat green arrows and pink ellipses"... How does this fit into your current thinking?

@alexcjohnson
Copy link
Collaborator

a template whereby ALL annotations and shapes would be styled

Good point, that's a different use case, I suspect it's important to support both. How about a parallel object like:

template.layout: {
    annotationdefaults: {arrowcolor: 'green'} // defaults applied to all annotations
    annotations: [{text: 'Confidential'}] // a specific annotation to add
}

Then to let users extract this from a reference plot... perhaps if we make name a real attribute of component array items, the first item with no name, if any, could become the default (kind of the inverse of the second option above), and only named annotations would be included in the array.

@nicolaskruchten
Copy link
Contributor

That could work. How are you looking at handling this in trace-land? Is there a way to style "all scatter traces" vs "the first scatter trace" etc?

@alexcjohnson
Copy link
Collaborator

I don't think there's ever a case where you want the template to add a whole trace to the plot - I was envisioning just cycling through traces per type.

@nicolaskruchten
Copy link
Contributor

Right, cycling makes sense for traces. I think we talked about this already and I'd forgotten, sorry :)

I guess that's not going to be good inspiration for annotations, shapes, images etc though.

@etpinard
Copy link
Contributor

I also like your 'alternative' at the end 👌


(Trying to mark down all possible use cases, this may not be important)

What if a user wants to define a colorway for annotation colors or maybe a sequence of line.dash values for shapes, could template.layout.(annotation|shape)defaults support array values?

@alexcjohnson
Copy link
Collaborator

could template.layout.(annotation|shape)defaults support array values?

I suppose it could - there are cases like the one-annotation-per-trace we've been looking at, though this seems a bit unusual. I'd vote to leave it out for now, but would be an easy non-breaking add later.

@etpinard
Copy link
Contributor

I'd vote to leave it out for now, but would be an easy non-breaking add later.

Fine by me 👍

@alexcjohnson alexcjohnson mentioned this issue Jun 27, 2018
2 tasks
@alexcjohnson
Copy link
Collaborator

We initially envisioned this system being limited to style attributes, meaning those marked with role: 'style'. But there are two problems with this:

  • By default we strip role out of our attributes (at the same time as we strip descriptions) - we could put it back, but that's a fairly big carry for just this application.
  • We haven't been very careful with role - personally I think I've probably erred toward marking things 'info' instead, if they might encode real information rather than just aesthetics - and it's not clear-cut anyway; users could easily have different expectations around which attributes should or should not be included.

Anyway, if something gets included in the template and you don't want it in subsequent plots, you have two chances to fix this: first after the template is created you can manually remove items; then when it's applied you can always override it in the new plot. So I'm thinking of ignoring role entirely. We will still omit valType: 'data_array' or arrayOk arrays, which is the most important differentiator between a template and a full plot.

Thoughts? Any attributes or attribute classes we really don't want in the template, that I should find some other way to exclude?

@etpinard
Copy link
Contributor

So I'm thinking of ignoring role entirely. We will still omit valType: 'data_array' or arrayOk arrays, which is the most important differentiator between a template and a full plot.

That sounds good to me 👌

@nicolaskruchten
Copy link
Contributor

What's the status of this issue? Is there an open PR?

@etpinard
Copy link
Contributor

Most of it was merged in #2761 and release in 1.39.0, but I suspect there's a few open items remaining and @alexcjohnson wanted to keep this ticket open to track them.

@alexcjohnson
Copy link
Collaborator

Oh, no we can close it. If bugs or additional desires emerge we can make a new issue.

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

No branches or pull requests

5 participants