[BUG] on_change events of DataTable CDS fail when source is set after init

[maeglin89273]

Software version info
bokeh 2.0.0
chrome 80.0.3987.132 (Official Build) (64-bit)
ubuntu 19.10

Description of expected behavior and the observed behavior
Context:

• An on_change('data', callback) is registered on the ColumnDataSource which is attached to a DataTable
• An on_change('indices', callback) is registered on the "selected" field of a ColumnDataSource which is attached to a DataTable

Expect:

• The callback will be called while the content in the DataTable is edited
• The callback will be called while a row in the DataTable is selected

Observed:

• Both callbacks are not called

Complete, minimal, self-contained example code that reproduces the issue

from bokeh.io import curdoc
from bokeh.models import TableColumn, ColumnDataSource, DataTable

columns = [TableColumn(field='a', title='a'),
             TableColumn(field='b', title='b'),
             TableColumn(field='c', title='c')]

cds = ColumnDataSource(dict(a=[1,2,3],b=[2,3,4], c=[3,4,5]))

table = DataTable(editable=True, width=300, height=150)
table.source = cds
table.columns = columns
def on_data(attr, old, new):
    print('on_data')

def on_select(attr, old, new):
    print('on_select')


table.source.on_change('data', on_data)
table.source.selected.on_change('indices', on_select)

curdoc().add_root(table)

[bryevdv]

@maeglin89273 your code works for me if I change to pass source directly to DataTable:

table = DataTable(editable=True, width=300, height=150, source=cds)

It should work the way you have it too, and I am not sure why it does not, but hopefully this is an immediate solution for you.

[maeglin89273]

Thanks, @bryevdv ! I didn't aware I can set the source and columns in the constructor. It really solves my problem.

[bryevdv]

@maeglin89273 FYI in general, every typed property on any bokeh object can always be configured via a keyword argument in the initializer.

[bryevdv]

OK, the custom initializer on the base class is the problem:

    def __init__(self, **kw):
        super().__init__(**kw)
        if "view" not in kw:
            self.view = CDSView(source=self.source)

If source is not passed during init, then self.source is None and that certainly will break CDSView. So, options:

• require source to be passed in init (raise an exception if not)
• don't configure a default CDSView if source is not passed in init
• always create an unconfigured CDSView and defer "wiring it up" until the JS side

The last option would allow us to get rid of the initializer altogether which would be good, but make BokehJS more complicated and also add an unnecessary backward sync to server apps, which is not great. At the further end of possibilities, we could add a "finalize" protocol for making these kinds of configurations right at serialization time (similar to validations) and that might allow getting rid of most of the custom initializers. That would allow us to still have the configuration done on the Python side and that would be ideal.

[bryevdv]

@mattpap for reference this solves the issue:

diff --git a/bokeh/document/util.py b/bokeh/document/util.py
index 3583323b2..ab012ec80 100644
--- a/bokeh/document/util.py
+++ b/bokeh/document/util.py
@@ -125,6 +125,7 @@ def references_json(references):

     references_json = []
     for r in references:
+        r._finalize()
         struct = r.struct
         struct['attributes'] = r._to_json_like(include_defaults=False)
         references_json.append(struct)
diff --git a/bokeh/model.py b/bokeh/model.py
index c6594fb1f..e5f17077e 100644
--- a/bokeh/model.py
+++ b/bokeh/model.py
@@ -774,6 +774,16 @@ class Model(HasProps, PropertyCallbackManager, EventCallbackManager):

         return html

+    def _finalize(self):
+        """ This methods can be oeverriden if  a model needs to defer some
+        configuration until serialization time. E.g. if a model needs to set
+        up a relation bewteen two sub-components.
+
+        WARNING: no new models should ever be created inside this method
+
+        """
+        pass
+
 #-----------------------------------------------------------------------------
 # Private API
 #-----------------------------------------------------------------------------
diff --git a/bokeh/models/widgets/tables.py b/bokeh/models/widgets/tables.py
index 9d2c5aa38..aabf093cd 100644
--- a/bokeh/models/widgets/tables.py
+++ b/bokeh/models/widgets/tables.py
@@ -610,16 +610,15 @@ class TableWidget(Widget):
     The source of data for the widget.
     """)

-    view = Instance(CDSView, help="""
+    view = Instance(CDSView, default=lambda: CDSView(), help="""
     A view into the data source to use when rendering table rows. A default view
     of the entire data source is created if a view is not passed in during
     initialization.
     """)

-    def __init__(self, **kw):
-        super().__init__(**kw)
-        if "view" not in kw:
-            self.view = CDSView(source=self.source)
+    def _finalize(self):
+        if self.view.source is None:
+            self.view.source = self.source

 class DataTable(TableWidget):
     ''' Two dimensional grid for visualisation and editing large amounts

Would you be in favor of this approach to get rid of initializers on models more generally?