ipython / traitlets

A lightweight Traits like module
https://traitlets.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
615 stars 201 forks source link

Collating list traits fails when only one is given - regression in traitlets 5.13.0 #908

Open krassowski opened 2 months ago

krassowski commented 2 months ago

See https://github.com/jupyterlab/jupyter-ai/issues/913. Introduced in https://github.com/ipython/traitlets/pull/885 which should have no side effects as it was meant to only update types.

krassowski commented 2 months ago

The regression is caused by a change in traitlets/traitlets.py file, in Instance class, in validate method:

-    def validate(self, obj, value):
+    def validate(self, obj: t.Any, value: t.Any) -> T | None:
         assert self.klass is not None
+        if self.allow_none and value is None:
+            return value
         if isinstance(value, self.klass):  # type:ignore[arg-type]
-            return value
+            return t.cast(T, value)
         else:
             self.error(obj, value)

Nothing obviously wrong here, but apparently the Container logic depends on the error being thrown here :shrug:

krassowski commented 2 months ago

The self.error() call was previously making the logic work correctly because the error is excepted to be raised by DeferredConfigString.get_value():

https://github.com/ipython/traitlets/blob/13de53c537a257402035139dd6862558eb19d362/traitlets/config/loader.py#L390-L417

get_value() calls trait.from_string():

https://github.com/ipython/traitlets/blob/13de53c537a257402035139dd6862558eb19d362/traitlets/traitlets.py#L3511-L3519

which calls validate().

So if we pass --ServerApp.a_list=a_value where a_list = List(allow_none=True) previously we would get a_list = ['a_value'] and now we are getting a_list = None.