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.
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])
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.
Yes the config is for configuring the Template extraction. I like the API you have provided and prefer makeTemplate over the word extract. Thanks!
Hey @bpostlethwaite this looks awesome.
I only have a few points to add right now:
Generally +1, with a few comments:
Plotly.buildTemplateFrom() and Plotly.buildFigureFromTemplate() or something?@nicolaskruchten sorry a more up-to-date current state is here: https://github.com/plotly/plotly.js/compare/master...plotlyjs_templating
Your second and third points still apply though!
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:
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:
extendDeep since these are plain JSON 馃帀 layout.template with this JSONsupplyDefaults, 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.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?
On further reflection, I believe the new scheme above might also be required in order to get Plotly.react () to work as expected :)
I can't think of any problems with @alexcjohnson's most recent comment. :ok_hand:
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 :burrito:
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:
{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.{} 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.
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?
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.
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?
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.
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.
I also like your 'alternative' at the end :ok_hand:
(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?
could
template.layout.(annotation|shape)defaultssupport 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.
I'd vote to leave it out for now, but would be an easy non-breaking add later.
Fine by me :+1:
We initially envisioned this system being limited to style attributes, meaning those marked with role: 'style'. But there are two problems with this:
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.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?
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 :ok_hand:
What's the status of this issue? Is there an open PR?
Most of it was merged in https://github.com/plotly/plotly.js/pull/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.
Oh, no we can close it. If bugs or additional desires emerge we can make a new issue.
Most helpful comment
Plotly.extractStylelooks great - I'm assumingconfigis potential config for the extraction operation itself, as opposed to the plot config as innewPlot? I might change the name toextractTemplateormakeTemplate, which would accommodate future expansion beyond style information.I don't think mutation itself (of
dataandlayout) is a requirement, just the ability to update an existing plot (replacinggd.dataandgd.layout)... so if we use:then we can follow it up with:
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
infoand decide if the templating worked as intended before going ahead and plugging the result into the plot.I would assume referenced is better and omit cloning until someone explicitly asks.