Repository: ipython/traitlets (python)

Issue:
Change to Set argv parsing behavior in 5.x?
I didn't see an existing issue for this yet, but with traitlets 5 release we had this reported in nbconvert: https://github.com/jupyter/nbconvert/issues/1410 related to set assignment from command line not working with the upgrade. Specifically `--TagRemovePreprocessor.remove_cell_tags={\"noeffect\"}` works with trailets 4 but not 5, though `[\"noeffect\"]` works in both for `remove_cell_tags = Set(Unicode())`.

Locally there isn't a problem with assigning from a set or a list inside a python process against a `Set` traitlet attribute. Tracing the issue further I got to https://github.com/jupyter/jupyter_core/blob/master/jupyter_core/application.py#L226 and it seems like the issue is in argv parsing changing behavior here.

Is there a new pattern expected for parse_command_line parsing for `Set` traits or now or is this a bug with this particular pattern?

Pull request description:
Fix deprecated from_string for Set, Tuple
- fix support for deprecated literals for tuples, sets on CLI
- fix from_string in general on Tuple

closes #625

Patch:
diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst
index 691a73d..5511e81 100644
--- a/docs/source/changelog.rst
+++ b/docs/source/changelog.rst
@@ -4,6 +4,12 @@ Changes in Traitlets
 Traitlets 5.0
 -------------
 
+5.0.5
+*****
+
+- Support deprecated literals for sets, tuples on the command-line: ``nbconvert --TagRemovePreprocessor.remove_cell_tags='{"tag"}'``
+- Fix ``from_string_list`` for Tuples in general
+
 5.0.4
 *****
 
diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py
index 8bc4e1f..b1894a4 100644
--- a/traitlets/traitlets.py
+++ b/traitlets/traitlets.py
@@ -2412,6 +2412,8 @@ class Container(Instance):
     _valid_defaults = SequenceTypes
     _trait = None
 
+    _literal_from_string_pairs = ("[]", "()")
+
     def __init__(self, trait=None, default_value=None, **kwargs):
         """Create a container trait type from a list, set, or tuple.
 
@@ -2517,20 +2519,34 @@ class Container(Instance):
             r = s_list[0]
             if r == "None" and self.allow_none:
                 return None
-            if (
-                (r[0] == '[' and r[-1] == ']') or
-                (r[0] == '(' and r[-1] == ')')
+            if len(r) >= 2 and any(
+                r.startswith(start) and r.endswith(end)
+                for start, end in self._literal_from_string_pairs
             ):
+                if self.this_class:
+                    clsname = self.this_class.__name__ + "."
+                else:
+                    clsname = ""
+
                 warn(
                     "--{0}={1} for containers is deprecated in traitlets 5.0. "
-                    "You can pass --{0} item ... multiple times to add items to a list.".format(
-                        self.name, r),
-                    FutureWarning
+                    "You can pass `--{0} item` ... multiple times to add items to a list.".format(
+                        clsname + self.name, r
+                    ),
+                    FutureWarning,
                 )
                 return self.klass(literal_eval(r))
-        return self.klass([self.item_from_string(s) for s in s_list])
+        sig = inspect.signature(self.item_from_string)
+        if "index" in sig.parameters:
+            item_from_string = self.item_from_string
+        else:
+            # backward-compat: allow item_from_string to ignore index arg
+            item_from_string = lambda s, index=None: self.item_from_string(s)
+        return self.klass(
+            [item_from_string(s, index=idx) for idx, s in enumerate(s_list)]
+        )
 
-    def item_from_string(self, s):
+    def item_from_string(self, s, index=None):
         """Cast a single item from a string
 
         Evaluated when parsing CLI configuration from a string
@@ -2596,6 +2612,8 @@ class Set(List):
     klass = set
     _cast_types = (tuple, list)
 
+    _literal_from_string_pairs = ("[]", "()", "{}")
+
     # Redefine __init__ just to make the docstring more accurate.
     def __init__(self, trait=None, default_value=None, minlen=0, maxlen=sys.maxsize,
                  **kwargs):
@@ -2687,13 +2705,24 @@ class Tuple(Container):
                 warn("Traits should be given as instances, not types (for example, `Int()`, not `Int`)"
                      " Passing types is deprecated in traitlets 4.1.",
                      DeprecationWarning, stacklevel=2)
-            t = trait() if isinstance(trait, type) else trait
-            self._traits.append(t)
+                trait = trait()
+            self._traits.append(trait)
 
         if self._traits and default_value is None:
             # don't allow default to be an empty container if length is specified
             args = None
-        super(Container,self).__init__(klass=self.klass, args=args, **kwargs)
+        super(Container, self).__init__(klass=self.klass, args=args, **kwargs)
+
+    def item_from_string(self, s, index):
+        """Cast a single item from a string
+
+        Evaluated when parsing CLI configuration from a string
+        """
+        if not self._traits or index >= len(self._traits):
+            # return s instead of raising index error
+            # length errors will be raised later on validation
+            return s
+        return self._traits[index].from_string(s)
 
     def validate_elements(self, obj, value):
         if not self._traits:


Test patch:
diff --git a/traitlets/tests/test_traitlets.py b/traitlets/tests/test_traitlets.py
index e4f456b..a5cdb2a 100644
--- a/traitlets/tests/test_traitlets.py
+++ b/traitlets/tests/test_traitlets.py
@@ -45,6 +45,7 @@ from traitlets import (
     Instance,
     TCPAddress,
     List,
+    Set,
     Tuple,
     ObjectName,
     DottedObjectName,
@@ -2787,12 +2788,69 @@ def test_list_from_string(s, expected):
         (["x"], ValueError, Integer()),
         (["1", "x"], ["1", "x"], Unicode()),
         (["None"], [None], Unicode(allow_none=True)),
+        (["None"], ["None"], Unicode(allow_none=False)),
     ],
 )
 def test_list_items_from_string(s, expected, value_trait):
     _from_string_test(List(value_trait), s, expected)
 
 
+@pytest.mark.parametrize(
+    "s, expected",
+    [
+        ("[]", set()),
+        ('[1, 2, "x"]', {1, 2, "x"}),
+        ('{1, 2, "x"}', {1, 2, "x"}),
+        (["1", "x"], {"1", "x"}),
+        (["None"], None),
+    ],
+)
+def test_set_from_string(s, expected):
+    _from_string_test(Set, s, expected)
+
+
+@pytest.mark.parametrize(
+    "s, expected, value_trait",
+    [
+        (["1", "2", "3"], {1, 2, 3}, Integer()),
+        (["x"], ValueError, Integer()),
+        (["1", "x"], {"1", "x"}, Unicode()),
+        (["None"], {None}, Unicode(allow_none=True)),
+    ],
+)
+def test_set_items_from_string(s, expected, value_trait):
+    _from_string_test(Set(value_trait), s, expected)
+
+
+@pytest.mark.parametrize(
+    "s, expected",
+    [
+        ("[]", ()),
+        ("()", ()),
+        ('[1, 2, "x"]', (1, 2, "x")),
+        ('(1, 2, "x")', (1, 2, "x")),
+        (["1", "x"], ("1", "x")),
+        (["None"], None),
+    ],
+)
+def test_tuple_from_string(s, expected):
+    _from_string_test(Tuple, s, expected)
+
+
+@pytest.mark.parametrize(
+    "s, expected, value_traits",
+    [
+        (["1", "2", "3"], (1, 2, 3), [Integer(), Integer(), Integer()]),
+        (["x"], ValueError, [Integer()]),
+        (["1", "x"], ("1", "x"), [Unicode()]),
+        (["None"], ("None",), [Unicode(allow_none=False)]),
+        (["None"], (None,), [Unicode(allow_none=True)]),
+    ],
+)
+def test_tuple_items_from_string(s, expected, value_traits):
+    _from_string_test(Tuple(*value_traits), s, expected)
+
+
 @pytest.mark.parametrize(
     "s, expected",
     [