make toolbar=True the default in save()

[Dr-Irv]

Reference issue #4515 . Making the toolbar default to True to correspond to past behavior.

Documentation issue is that the docs for 1.13.3 are not published on the web site, so presumably if they were to get rebuilt the option would show up there.

[philippjfr]

Documentation issue is that the docs for 1.13.3 are not published on the web site

This is not true (e.g. you can see the release notes for 1.13.3 here: http://holoviews.org/releases.html), I think we just neglected to mention this in the docs.

I also don't think the default should be true as a picture of a toolbar is not useful even if it is useful for docs or other scenarios. I'm sorry about the regression in a micro-release which shouldn't have happened but I'm -1 on reverting to the old behavior. Cc: @jbednar Any opinion?

[philippjfr]

Also thanks for the PR, I appreciate the effort even though we disagree on the change.

[Dr-Irv]

This is not true (e.g. you can see the release notes for 1.13.3 here: http://holoviews.org/releases.html), I think we just neglected to mention this in the docs.

See this PR #4519 - if you go to the docs for the util module, the toolbar option is not shown for save()

[Dr-Irv]

I also don't think the default should be true as a picture of a toolbar is not useful even if it is useful for docs or other scenarios. I'm sorry about the regression in a micro-release which shouldn't have happened but I'm -1 on reverting to the old behavior. Cc: @jbednar Any opinion?

But it's NOT just for a picture! I use it to serve up the HTML that allows a user to interact with the visualization. The examples that are provided for deploying with Flask and Bokeh use this as well. I think you should keep the OLD default behavior and if someone needs to have it for a PNG, they can turn the toolbar off.

[philippjfr]

Oh sorry in that case I'd consider this a bug. The intended behaviour was to only enable this for PNG exports not for static HTML renders.

[Dr-Irv]

Oh sorry in that case I'd consider this a bug. The intended behaviour was to only enable this for PNG exports not for static HTML renders.

So how do you want to handle? Accept this PR as is, and create a new issue relative to making the behavior dependent on the type, and a follow-up PR? Or fix that "bug" in this PR by testing for the 'PNG' format? IMHO, I think that documenting that behavior could be confusing, so you're better off just having the toolbar argument, and if someone has a PNG, they just set toolbar=False .

[jbednar]

As suggested in #4515, I vote for having the toolbar suppressed by default for PNG, SVG, or other static image formats, and enabled by default for "live" formats like HTML. I think that's what nearly everyone will want, so that not many people will ever need to end up reading the docs for how to enable the opposite behavior.

[Dr-Irv]

As suggested in #4515, I vote for having the toolbar suppressed by default for PNG, SVG, or other static image formats, and enabled by default for "live" formats like HTML. I think that's what nearly everyone will want, so that not many people will ever need to end up reading the docs for how to enable the opposite behavior.

So I could modify this PR to have that behavior, and then should we remove the toolbar parameter to save() ? If not, then I could implement the following behavior:

• toolbar=None : default behavior based on the fmt parameter
• toolbar=True : toolbar appears, independent of fmt
• toolbar=False: no toolbar, independent of fmt

[philippjfr]

That seems good, I'd go with something like this for the condition:

if not toolbar and backend == 'bokeh' and (fmt == 'png' or (isinstance(filename, str) and filename.endswith('png'))):
    obj = obj.opts(toolbar=None, backend='bokeh', clone=True)

[philippjfr]

This also seems like a fairly annoying regression after you've pointed out that this indeed affects all static save output so I'd favor a 1.13.4 release sooner rather than later.

[philippjfr]

Thanks for being patient with me despite being confused about the actual issue @Dr-Irv and thanks for the PR!