search

holoviz / pyviz_comms

Bidirectional communication for the HoloViz ecosystem
BSD 3-Clause "New" or "Revised" License
32 stars 15 forks source link

[BUG] debounce js code doesn't do what it's supposed to do. #37

Closed arabidopsis closed 4 years ago

arabidopsis commented 5 years ago

The problem is at line 183 of pviz_comms.__init__.py

event_name = cb_obj.event_name

event_name is actually undefined... there is no event_name property on a widget I think cb_obj can be a BokehEvent (in which case it does have an event_name) or a Widget which doesn't.

With all events now "named" undefined if any two different events come within the debounce period all but the last will be ejected.

This showed up when I tried to adapt panel.models.widgets.FileInput widget to also return the filename. Only one of value/filename was ever returned.

My "solution" was to replace this line with var event_name = '{change}'; and then hack panel.io.notebook.get_comm_customjs to supply the change variable to the format of JS_CALLBACK.

I would turn this into a pull request but it requires updating two repos.... I'm not sure exposing a blob of (partial) JS code is the best design.

Also the unique_events function needs to have var data, event; added to it to prevent the data variable stomping on the enclosing scope's data variable (at least how panel monkeypatches the JS blob)

I'm using the current HEAD versions of both pyviz_comms and panel....

arabidopsis commented 5 years ago

I made a pull request that fixes the issue by creating an event_name out of the keys of the data object. This should me that it will work for both panel and holoviews and the key will distinguish between properties that have changed (at least for panel and by the looks holoviews also... better than having everything "undefined" anyhow).

philippjfr commented 5 years ago

I'm not sure exposing a blob of (partial) JS code is the best design.

This is definitely true, we decided to take that route because duplicating the partial JS blobs across multiple projects seemed even worse. Eventually we hope to actually build a small pyviz.js and/or perhaps encapsulate everything inside bokeh models. The current situation is certainly very suboptimal.

Your fix looks fine to me, but let me know if you foresee any other issues.

philippjfr commented 4 years ago

Since Panel 0.9.x and HoloViews 1.13.x we have now moved most of the JS code to a CommManager bokeh model in Panel.