Breaking change in ipykernel 6.18.0 with traitlets

[kylebarron]

We use a CI setup derived from here in the widget-ts-cookiecutter repo, and our CI tests started failing today with ipykernel 6.18.0.

Specifically, we get an exception of

>       if obj._cross_validation_lock is False:
E       AttributeError: 'MockComm' object has no attribute '_cross_validation_lock'

/opt/hostedtoolcache/Python/3.8.14/x64/lib/python3.8/site-packages/traitlets/traitlets.py:722: AttributeError

which is coming from traitlets here.

This is a private method, so maybe traitlets was in the wrong for relying on it. Presumably this happens now because of the comm changes in ipykernel in 6.18. Should I make an issue in traitlets instead?

[kylebarron]

Specifically, in 6.17.1, the ipywidgets.comm.Comm class hierarchy was

In [4]: ipykernel.comm.Comm.__mro__
Out[4]:
(ipykernel.comm.comm.Comm,
 traitlets.config.configurable.LoggingConfigurable,
 traitlets.config.configurable.Configurable,
 traitlets.traitlets.HasTraits,
 traitlets.traitlets.HasDescriptors,
 object)

while in 6.18, the class hierarchy is

In [5]: ipykernel.comm.Comm.__mro__
Out[5]: (ipykernel.comm.comm.Comm, comm.base_comm.BaseComm, object)

so at least in our tests, it looks like traitlets is expecting the comm object to have properties defined on both HasTraits and HasDescriptors

[blink1073]

cc @martinRenou, it looks like a change is needed in the comm package

[blink1073]

cc @ccordoba12, looks like you might need a temporary pin

[maartenbreddels]

I think the Comm implementation in ipykernel should derive from LoggingConfigurable, as should the CommManager.

I think @martinRenou has an implementation of that.

[blink1073]

I just yanked 6.18.0. We can release 6.18.1 once we sort this out.

[blink1073]

@martinRenou I think if the intent is to use the comm package directly in ipywidgets, we have to make it a traitlets subclass in the comm package itself.

[maximlt]

On a possibly related note (I need to dig more into it, as Kyle did), we now get the following error when importing the HoloViews extension in a notebook:

  File "/Users/mliquet/miniconda3/envs/testbughvplot/lib/python3.8/site-packages/holoviews/ipython/__init__.py", line 149, in __call__
    ip.run_line_magic('config', 'IPCompleter.use_jedi = False')
  File "/Users/mliquet/miniconda3/envs/testbughvplot/lib/python3.8/site-packages/IPython/core/interactiveshell.py", line 2364, in run_line_magic
    result = fn(*args, **kwargs)
  File "/Users/mliquet/miniconda3/envs/testbughvplot/lib/python3.8/site-packages/IPython/core/magics/config.py", line 163, in config
    configurables = sorted(set([ c for c in self.shell.configurables
  File "/Users/mliquet/miniconda3/envs/testbughvplot/lib/python3.8/site-packages/IPython/core/magics/config.py", line 164, in <listcomp>
    if c.__class__.class_traits(config=True)
AttributeError: type object 'CommManager' has no attribute 'class_traits'

I just yanked 6.18.0. We can release 6.18.1 once we sort this out.

ipykernel 6.18.0 has been released on conda-forge, can it be labelled as broken?

[martinRenou]

Thanks for pinging me @blink1073 I'll push on the new comm package to make sure to fix those.

[martinRenou]

@kylebarron would you be able to provide the entire traceback for your error AttributeError: 'MockComm' object has no attribute '_cross_validation_lock'?

I'd be curious to understand why validate is being called on the Comm object.

I think the Comm implementation in ipykernel should derive from LoggingConfigurable, as should the CommManager.

@maartenbreddels I fear this will not be sufficient and we'll need to inherit from HasTraits as @blink1073 and @kylebarron said.

On a possibly related note (I need to dig more into it, as Kyle did), we now get the following error when importing the HoloViews extension in a notebook:

@maximlt Do you know why the CommManager is being configured, and where it is being done? Having CommManager inherit from HasTrait might not be sufficient in this case if the configuration is crucial, and we might need to bring back all the traits definitions for the manager (cc. @maartenbreddels on this).

[philippjfr]

The removal of LoggingConfigurable as a baseclass also broke ipywidgets_bokeh because parent is no longer a valid argument to the constructor of CommManager, see bokeh/ipywidgets_bokeh#69 (comment)

[kylebarron]

would you be able to provide the entire traceback for your error AttributeError: 'MockComm' object has no attribute '_cross_validation_lock'?

It appears this happens whenever a new traitlets item is being set on a comm subclass? Here's a minimal repro case:

from ipykernel.comm import Comm
from traitlets import Unicode

class MockComm(Comm):
    comm_id = Unicode("a-b-c-d")

MockComm()

> python repro.py
Traceback (most recent call last):
  File "repro.py", line 7, in <module>
    MockComm()
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/ipykernel/comm/comm.py", line 18, in __init__
    super().__init__(*args, **kwargs)
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/comm/base_comm.py", line 39, in __init__
    self.comm_id = comm_id if comm_id else uuid.uuid4().hex
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 715, in __set__
    self.set(obj, value)
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 689, in set
    new_value = self._validate(obj, value)
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/traitlets/traitlets.py", line 722, in _validate
    if obj._cross_validation_lock is False:
AttributeError: 'MockComm' object has no attribute '_cross_validation_lock'
Exception ignored in: <function BaseComm.__del__ at 0x106d354c0>
Traceback (most recent call last):
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/comm/base_comm.py", line 64, in __del__
    self.close(deleting=True)
  File "/Users/kbarron/unfolded/platform/python/modules/map-sdk/env/lib/python3.8/site-packages/comm/base_comm.py", line 95, in close
    if self._closed:
AttributeError: 'MockComm' object has no attribute '_closed'

Maybe setting a traitlets value on a subclass like this wasn't supposed to work and isn't the intended API?