Repository: ipython/traitlets
Pull request #616 (2020-09-04 08:31:01)

nicer error message for multiply defined config
Original error is displayed. Now looks like:

```
Error loading argument App.opt=['1', '2'], opt only accepts one value, got 2: ['1', '2']
```

related to #614 

Leaving #614 open to the possibility of changing the behavior of multiply defined scalar options to accept the last one, which was the de facto behavior of traitlets 4.


Patch:
diff --git a/traitlets/config/loader.py b/traitlets/config/loader.py
index 11599ea..73d4959 100644
--- a/traitlets/config/loader.py
+++ b/traitlets/config/loader.py
@@ -5,14 +5,11 @@
 
 import argparse
 import copy
-import functools as fnt
 import os
 import re
 import sys
 import json
 import warnings
-from ast import literal_eval
-from collections import defaultdict
 
 from ..utils import cast_unicode
 
@@ -366,6 +363,10 @@ class DeferredConfig:
     def get_value(self, trait):
         raise NotImplementedError("Implement in subclasses")
 
+    def _super_repr(self):
+        # explicitly call super on direct parent
+        return super(self.__class__, self).__repr__()
+
 
 class DeferredConfigString(str, DeferredConfig):
     """Config value for loading config from a string
@@ -396,8 +397,7 @@ class DeferredConfigString(str, DeferredConfig):
             return s
 
     def __repr__(self):
-        super_repr = super(DeferredConfigString, self).__repr__()
-        return '%s(%s)' % (self.__class__.__name__, super_repr)
+        return '%s(%s)' % (self.__class__.__name__, self._super_repr())
 
 
 class DeferredConfigList(list, DeferredConfig):
@@ -423,7 +423,7 @@ class DeferredConfigList(list, DeferredConfig):
         else:
             # only allow one item
             if len(self) > 1:
-                raise ValueError("Multiply defined option: %s" % trait.name)
+                raise ValueError(f"{trait.name} only accepts one value, got {len(self)}: {list(self)}")
             src = self[0]
             cast = trait.from_string
 
@@ -436,8 +436,7 @@ class DeferredConfigList(list, DeferredConfig):
             return src
 
     def __repr__(self):
-        super_repr = super(DeferredConfigList, self).__repr__()
-        return '%s(%s)' % (self.__class__.__name__, super_repr)
+        return '%s(%s)' % (self.__class__.__name__, self._super_repr())
 
 
 #-----------------------------------------------------------------------------
@@ -1019,8 +1018,12 @@ class KVArgParseConfigLoader(ArgParseConfigLoader):
             # eval the KV assignment
             try:
                 self._exec_config_str(lhs, rhs, trait)
-            except Exception:
-                raise ArgumentError("Invalid argument: '%s=%s'" % (lhs, rhs))
+            except Exception as e:
+                # cast deferred to nicer repr for the error
+                # DeferredList->list, etc
+                if isinstance(rhs, DeferredConfig):
+                    rhs = rhs._super_repr()
+                raise ArgumentError(f"Error loading argument {lhs}={rhs}, {e}")
 
         for subc in self.parsed_data._flags:
             self._load_flag(subc)


Test patch:
diff --git a/traitlets/config/tests/test_application.py b/traitlets/config/tests/test_application.py
index 77c623f..f0b5540 100644
--- a/traitlets/config/tests/test_application.py
+++ b/traitlets/config/tests/test_application.py
@@ -16,6 +16,7 @@ from io import StringIO
 from tempfile import TemporaryDirectory
 from unittest import TestCase
 
+import pytest
 from pytest import mark
 
 from traitlets import (
@@ -628,6 +629,23 @@ class TestApplication(TestCase):
             self.assertEqual(app.running, False)
 
 
+def test_cli_multi_scalar(caplog):
+    class App(Application):
+        aliases = {"opt": "App.opt"}
+        opt = Unicode(config=True)
+
+    app = App(log=logging.getLogger())
+    with pytest.raises(SystemExit):
+        app.parse_command_line(["--opt", "1", "--opt", "2"])
+    record = caplog.get_records("call")[-1]
+    message = record.message
+
+    assert "Error loading argument" in message
+    assert "App.opt=['1', '2']" in message
+    assert "opt only accepts one value" in message
+    assert record.levelno == logging.CRITICAL
+
+
 class Root(Application):
     subcommands = {
         'sub1': ('traitlets.config.tests.test_application.Sub1', 'import string'),
@@ -691,7 +709,7 @@ def test_show_config(capsys):
     cfg.MyApp.i = 5
     # don't show empty
     cfg.OtherApp
-    
+
     app = MyApp(config=cfg, show_config=True)
     app.start()
     out, err = capsys.readouterr()
@@ -704,7 +722,7 @@ def test_show_config_json(capsys):
     cfg = Config()
     cfg.MyApp.i = 5
     cfg.OtherApp
-    
+
     app = MyApp(config=cfg, show_config_json=True)
     app.start()
     out, err = capsys.readouterr()