[BUG] TypeError after replacing tools on a toolbar

[p-himik]

My plot should have different sets of tools depending on what is displayed.
setting plot.toolbar doesn't help since the toolbar model change not not handled correctly.
Executing something like plot.toolbar.tools = new_tools_list seems to work but results in this exception in JS console:

bokeh.js:30582 Uncaught TypeError: Cannot read property 'el' of undefined
    at el (bokeh.js:30582)
    at Array.map (<anonymous>)
    at ToolbarBaseView.render (bokeh.js:30586)
    at ToolbarPanelView.render (bokeh.js:11764)
    at PlotView._paint_levels (bokeh.js:22064)
    at PlotView.paint (bokeh.js:22044)
    at PlotView.repaint (bokeh.js:21983)
    at PlotView.<anonymous> (bokeh.js:21892)
    at Signal0.Signal.emit (bokeh.js:4167)
    at Signal0.emit (bokeh.js:4180)

Both Toolbar and ToolbarBaseView attach a signal on the tools property change. However, the Toolbar.gestures property change does not remove the old references:

// Toolbar._init_tools()
if (!some(this.gestures[et].tools, (t) => t.id == tool.id))
  this.gestures[et].tools = this.gestures[et].tools.concat([tool])

Because of this, only the addition of tools works without errors. And while gestures is a property in BokehJS, it is not in the Python counterpart.

Similar code can be seen in ProxyToolbar but it doesn't handle the changes to the list of tools at all, so I guess it was never on the plate.

While it's not reflected in the error above, other attributes have the same issue of not being pruned on tools change: inspectors, help, actions.

[p-himik]

As a workaround, something like this seems to work, at least in my case.

from bokeh.models import Toolbar


class MutableToolbar(Toolbar):
    __implementation__ = "mutable_toolbar.ts"


f = figure(toolbar=MutableToolbar(tools=[...]))

import {Toolbar} from "models/tools/toolbar"
import {Tool} from "models/tools";
import {Set} from "core/util/data_structures"

export class MutableToolbar extends Toolbar {
    protected _init_tools(): void {
        super._init_tools();
        const all_ids = new Set(this.tools.map(t => t.id));
        const tool_exists = ((t: Tool | null) => t && all_ids.has(t.id));
        for (const k in this.gestures) {
            const gts = (this.gestures as any)[k];
            gts.tools = gts.tools.filter(tool_exists);
            if (!tool_exists(gts.active)) {
                gts.active = null;
            }
        }
        this.actions = this.actions.filter(tool_exists);
        this.inspectors = this.inspectors.filter(tool_exists);
        this.help = this.help.filter(tool_exists);
    }
}

[bryevdv]

@p-himik I am pretty maxed out on bandwidth. If you have a workable solution that is not very intrusive *(which it looks like you do?) then a PR would be welcome.

[p-himik]

It will be a bit more intrusive since a proper fix would also change ProxyToolbar. I'll come up with a PR soon.

[p-himik]

OK. First of all, I hate TS. With passion. 95% of the time has been spent chasing obscure type errors. Understanding and debugging C++ w/boost templates compilation errors was more fun.

Second of all, the tests hate me.
For example, among dozens of other errors, I get this one: [...]
Update: nevermind, apparently my brain has stopped working an hour ago. Will create the PR after fixing the tests.

[bryevdv]

OK. First of all, I hate TS. With passion

Sorry you are not having a great experience with it. I can state that just the process of porting to it caught several latent bugs and errors by itself, and since the port I've definitely been prevented from checking in bugs by it. Thanks for your patience in working with it, and if there is something we can do to help make things smoother please let us know.

[p-himik]

Seems to work fine now.

@bryevdv Oh no, sorry for that rant. I completely get the virtues TS provides for projects similar to Bokeh (big code base, multiple contributors from different backgrounds). It's just that I'm mostly used to writing code for other kinds of projects, with no such needs.