Repository: IDSIA/sacred
Pull request #386 (2018-11-17 03:52:59)

Add SignatureError
Adds a `SignatureError` that is raised when the passed config and arguments do not match the signature of a captured function. It inherits from `TypeError`, so any check for this still works and is consistent with Python errors.
Also adds more advanced options for `filter_traceback` that allow `SacredErrors` to always (except for debugging) suppress sacred internal stacktrace, even if the Exception was raised from within sacred. This now applies to the new `SignatureError` that is raised from within sacred, but the point of interest is the calling function.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/20156743/badge)](https://coveralls.io/builds/20156743)

Coverage increased (+0.02%) to 83.622% when pulling **45808eab803bfa5f4822b6409ee55482f0dea938 on thequilo:improved-error-messages/signature-error** into **94785b9592b16e8498212ad9cfa0e05d49a1819d on IDSIA:master**.


Patch:
diff --git a/sacred/config/signature.py b/sacred/config/signature.py
index 1e6b0ea..a63a0e4 100644
--- a/sacred/config/signature.py
+++ b/sacred/config/signature.py
@@ -6,7 +6,7 @@ import inspect
 from collections import OrderedDict
 import sys
 
-from sacred.utils import MissingConfigError
+from sacred.utils import MissingConfigError, SignatureError
 
 if sys.version_info[0] < 3:  # python2
     def get_argspec(f):
@@ -127,7 +127,7 @@ class Signature(object):
     def _assert_no_unexpected_args(self, expected_args, args):
         if not self.vararg_name and len(args) > len(expected_args):
             unexpected_args = args[len(expected_args):]
-            raise TypeError("{} got unexpected argument(s): {}".format(
+            raise SignatureError("{} got unexpected argument(s): {}".format(
                 self.name, unexpected_args))
 
     def _assert_no_unexpected_kwargs(self, expected_args, kwargs):
@@ -135,15 +135,16 @@ class Signature(object):
             return
         unexpected_kwargs = set(kwargs) - set(expected_args)
         if unexpected_kwargs:
-            raise TypeError("{} got unexpected kwarg(s): {}".format(
+            raise SignatureError("{} got unexpected kwarg(s): {}".format(
                 self.name, sorted(unexpected_kwargs)))
 
     def _assert_no_duplicate_args(self, expected_args, args, kwargs):
         positional_arguments = expected_args[:len(args)]
         duplicate_arguments = [v for v in positional_arguments if v in kwargs]
         if duplicate_arguments:
-            raise TypeError("{} got multiple values for argument(s) {}".format(
-                self.name, duplicate_arguments))
+            raise SignatureError(
+                "{} got multiple values for argument(s) {}".format(
+                    self.name, duplicate_arguments))
 
     def _fill_in_options(self, args, kwargs, options, bound):
         free_params = self.get_free_parameters(args, kwargs, bound)
diff --git a/sacred/utils.py b/sacred/utils.py
index d41dd66..01aff45 100755
--- a/sacred/utils.py
+++ b/sacred/utils.py
@@ -89,10 +89,15 @@ class TimeoutInterrupt(SacredInterrupt):
 
 
 class SacredError(Exception):
-    def __init__(self, message, print_traceback=True, filter_traceback=True,
-                 print_usage=False):
+    def __init__(self, message, print_traceback=True,
+                 filter_traceback='default', print_usage=False):
         super(SacredError, self).__init__(message)
         self.print_traceback = print_traceback
+        if filter_traceback not in ['always', 'default', 'never']:
+            raise ValueError(
+                'filter_traceback must be one of \'always\', '
+                '\'default\' or \'never\', not ' + filter_traceback
+            )
         self.filter_traceback = filter_traceback
         self.print_usage = print_usage
 
@@ -114,7 +119,7 @@ class CircularDependencyError(SacredError):
 
     def __init__(self, message='Circular dependency detected:',
                  ingredients=None, print_traceback=True,
-                 filter_traceback=True, print_usage=False):
+                 filter_traceback='default', print_usage=False):
         super(CircularDependencyError, self).__init__(
             message,
             print_traceback=print_traceback,
@@ -139,7 +144,7 @@ class ConfigError(SacredError):
     def __init__(self, message, conflicting_configs=(),
                  print_conflicting_configs=True,
                  print_traceback=True,
-                 filter_traceback=True, print_usage=False,
+                 filter_traceback='default', print_usage=False,
                  config=None):
         super(ConfigError, self).__init__(message,
                                           print_traceback=print_traceback,
@@ -212,9 +217,9 @@ class MissingConfigError(SacredError):
 
     def __init__(self, message='Configuration values are missing:',
                  missing_configs=(),
-                 print_traceback=False, filter_traceback=True,
+                 print_traceback=False, filter_traceback='default',
                  print_usage=True):
-        message = '{}: {}'.format(message, missing_configs)
+        message = '{} {}'.format(message, missing_configs)
         super(MissingConfigError, self).__init__(
             message, print_traceback=print_traceback,
             filter_traceback=filter_traceback, print_usage=print_usage
@@ -227,7 +232,7 @@ class NamedConfigNotFoundError(SacredError):
     def __init__(self, named_config, message='Named config not found:',
                  available_named_configs=(),
                  print_traceback=False,
-                 filter_traceback=True, print_usage=False):
+                 filter_traceback='default', print_usage=False):
         message = '{} "{}". Available config values are: {}'.format(
             message, named_config, available_named_configs)
         super(NamedConfigNotFoundError, self).__init__(
@@ -247,7 +252,7 @@ class ConfigAddedError(ConfigError):
                  message='Added new config entry that is not used anywhere',
                  captured_args=(),
                  print_conflicting_configs=True, print_traceback=False,
-                 filter_traceback=True, print_usage=False,
+                 filter_traceback='default', print_usage=False,
                  print_suggestions=True,
                  config=None):
         super(ConfigAddedError, self).__init__(
@@ -271,6 +276,18 @@ class ConfigAddedError(ConfigError):
         return s
 
 
+class SignatureError(SacredError, TypeError):
+    """
+    Error that is raised when the passed arguments do not match the functions
+    signature
+    """
+    def __init__(self, message, print_traceback=True,
+                 filter_traceback='always',
+                 print_usage=False):
+        super(SignatureError, self).__init__(
+            message, print_traceback, filter_traceback, print_usage)
+
+
 def create_basic_stream_logger():
     logger = logging.getLogger('')
     logger.setLevel(logging.INFO)
@@ -456,42 +473,59 @@ def _is_sacred_frame(frame):
     return frame.f_globals["__name__"].split('.')[0] == 'sacred'
 
 
-def print_filtered_stacktrace(filter_traceback=True):
+def print_filtered_stacktrace(filter_traceback='default'):
     print(format_filtered_stacktrace(filter_traceback), file=sys.stderr)
 
 
-def format_filtered_stacktrace(filter_traceback=True):
+def format_filtered_stacktrace(filter_traceback='default'):
+    """
+    Returns the traceback as `string`.
+
+    `filter_traceback` can be one of:
+        - 'always': always filter out sacred internals
+        - 'default': Default behaviour: filter out sacred internals
+                if the exception did not originate from within sacred, and
+                print just the internal stack trace otherwise
+        - 'never': don't filter, always print full traceback
+        - All other values will fall back to 'never'.
+    """
     exc_type, exc_value, exc_traceback = sys.exc_info()
     # determine if last exception is from sacred
     current_tb = exc_traceback
     while current_tb.tb_next is not None:
         current_tb = current_tb.tb_next
 
-    if filter_traceback:
-        if _is_sacred_frame(current_tb.tb_frame):
-            header = ["Exception originated from within Sacred.\n"
-                      "Traceback (most recent calls):\n"]
-            texts = tb.format_exception(exc_type, exc_value, current_tb)
-            return ''.join(header + texts[1:]).strip()
+    if filter_traceback == 'default' \
+            and _is_sacred_frame(current_tb.tb_frame):
+        # just print sacred internal trace
+        header = ["Exception originated from within Sacred.\n"
+                  "Traceback (most recent calls):\n"]
+        texts = tb.format_exception(exc_type, exc_value, current_tb)
+        return ''.join(header + texts[1:]).strip()
+    elif filter_traceback in ('default', 'always'):
+        # print filtered stacktrace
+        if sys.version_info >= (3, 5):
+            tb_exception = \
+                tb.TracebackException(exc_type, exc_value, exc_traceback,
+                                      limit=None)
+            return ''.join(filtered_traceback_format(tb_exception))
         else:
-            if sys.version_info >= (3, 5):
-                tb_exception = \
-                    tb.TracebackException(exc_type, exc_value, exc_traceback,
-                                          limit=None)
-                return ''.join(filtered_traceback_format(tb_exception))
-            else:
-                s = "Traceback (most recent calls WITHOUT Sacred internals):"
-                current_tb = exc_traceback
-                while current_tb is not None:
-                    if not _is_sacred_frame(current_tb.tb_frame):
-                        tb.print_tb(current_tb, 1)
-                    current_tb = current_tb.tb_next
-                s += "\n".join(tb.format_exception_only(exc_type,
-                                                        exc_value)).strip()
-                return s
-    else:
+            s = "Traceback (most recent calls WITHOUT Sacred internals):"
+            current_tb = exc_traceback
+            while current_tb is not None:
+                if not _is_sacred_frame(current_tb.tb_frame):
+                    tb.print_tb(current_tb, 1)
+                current_tb = current_tb.tb_next
+            s += "\n".join(tb.format_exception_only(exc_type,
+                                                    exc_value)).strip()
+            return s
+    elif filter_traceback == 'never':
+        # print full stacktrace
         return '\n'.join(
             tb.format_exception(exc_type, exc_value, exc_traceback))
+    else:
+        raise ValueError('Unknown value for filter_traceback: ' +
+                         filter_traceback)
 
 
 def format_sacred_error(e, short_usage):


Test patch:
diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py
index 2e4e61a..5511c6f 100644
--- a/tests/test_exceptions.py
+++ b/tests/test_exceptions.py
@@ -64,7 +64,14 @@ def test_format_filtered_stacktrace_true():
     try:
         f()
     except:
-        st = format_filtered_stacktrace(filter_traceback=True)
+        st = format_filtered_stacktrace(filter_traceback='default')
+        assert 'captured_function' not in st
+        assert 'WITHOUT Sacred internals' in st
+
+    try:
+        f()
+    except:
+        st = format_filtered_stacktrace(filter_traceback='always')
         assert 'captured_function' not in st
         assert 'WITHOUT Sacred internals' in st
 
@@ -79,24 +86,29 @@ def test_format_filtered_stacktrace_false():
     try:
         f()
     except:
-        st = format_filtered_stacktrace(filter_traceback=False)
+        st = format_filtered_stacktrace(filter_traceback='never')
         assert 'captured_function' in st
 
 
 @pytest.mark.parametrize(
     'print_traceback,filter_traceback,print_usage,expected', [
-        (False, False, False, '.*SacredError: message'),
-        (True, False, False, r'Traceback \(most recent call last\):\n*'
-                             r'\s*File ".*", line \d*, in '
-                             r'test_format_sacred_error\n*'
-                             r'.*\n*'
-                             r'.*SacredError: message'),
-        (False, True, False, r'.*SacredError: message'),
-        (False, False, True, r'usage\n.*SacredError: message'),
-        (True, True, False, r'Traceback \(most recent calls WITHOUT Sacred '
-                            r'internals\):\n*'
-                            r'(\n|.)*'
-                            r'.*SacredError: message')
+        (False, 'never', False, '.*SacredError: message'),
+        (True, 'never', False, r'Traceback \(most recent call last\):\n*'
+                               r'\s*File ".*", line \d*, in '
+                               r'test_format_sacred_error\n*'
+                               r'.*\n*'
+                               r'.*SacredError: message'),
+        (False, 'default', False, r'.*SacredError: message'),
+        (False, 'always', False, r'.*SacredError: message'),
+        (False, 'never', True, r'usage\n.*SacredError: message'),
+        (True, 'default', False, r'Traceback \(most recent calls WITHOUT '
+                                 r'Sacred internals\):\n*'
+                                 r'(\n|.)*'
+                                 r'.*SacredError: message'),
+        (True, 'always', False, r'Traceback \(most recent calls WITHOUT '
+                                r'Sacred internals\):\n*'
+                                r'(\n|.)*'
+                                r'.*SacredError: message')
     ])
 def test_format_sacred_error(print_traceback, filter_traceback, print_usage,
                              expected):
@@ -105,5 +117,4 @@ def test_format_sacred_error(print_traceback, filter_traceback, print_usage,
                           print_usage)
     except SacredError as e:
         st = format_sacred_error(e, 'usage')
-        print(st)
         assert re.match(expected, st, re.MULTILINE)