Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
krassowski opened this issue Jul 26, 2024 · 2 comments

Comments

@krassowski
Copy link
Member

See jupyterlab/jupyter-ai#913. Introduced in #885 which should have no side effects as it was meant to only update types.

@krassowski
Copy link
Member Author

krassowski commented Jul 26, 2024

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 🤷

@krassowski
Copy link
Member Author

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

class DeferredConfigString(str, DeferredConfig):
"""Config value for loading config from a string
Interpretation is deferred until it is loaded into the trait.
Subclass of str for backward compatibility.
This class is only used for values that are not listed
in the configurable classes.
When config is loaded, `trait.from_string` will be used.
If an error is raised in `.from_string`,
the original string is returned.
.. versionadded:: 5.0
"""
def get_value(self, trait: TraitType[t.Any, t.Any]) -> t.Any:
"""Get the value stored in this string"""
s = str(self)
try:
return trait.from_string(s)
except Exception:
# exception casting from string,
# let the original string lie.
# this will raise a more informative error when config is loaded.
return s

get_value() calls trait.from_string():

def from_string(self, s: str) -> T | None:
"""Load value from a single string"""
if not isinstance(s, str):
raise TraitError(f"Expected string, got {s!r}")
try:
test = literal_eval(s)
except Exception:
test = None
return self.validate(None, test)

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant