Better warnings (#838)Co-authored-by: pre-commit-ci[bot] <66853113+pr…

…e-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Steven Silvester <steven.silvester@ieee.org> * Replace FutureWarnings with DeprecationWarnings per https://docs.python.org/3/library/warnings.html#warning-categories, FutureWarnings are intended for end users, and DeprecationWarnings are for programmers. We are targetting programmers here. This is a step towards fixing #837 * ref: remove unused _names_re this is private and unused. * feat: Always include stacklevel when warn()ing Fixes #837 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fix import * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * lint --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Steven Silvester <steven.silvester@ieee.org>
ipython · Apr 7, 2023 · 5cbf807 · 5cbf807
1 parent 21fdc40
commit 5cbf807
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 67 deletions.
diff --git a/traitlets/__init__.py b/traitlets/__init__.py
@@ -1,12 +1,11 @@
 """Traitlets Python configuration system"""
-from warnings import warn
-
 from . import traitlets
 from ._version import __version__, version_info
 from .traitlets import *
 from .utils.bunch import Bunch
 from .utils.decorators import signature_has_traits
 from .utils.importstring import import_item
+from .utils.warnings import warn
 
 __all__ = [
     "traitlets",

diff --git a/traitlets/config/configurable.py b/traitlets/config/configurable.py
@@ -5,7 +5,6 @@
 
 
 import logging
-import warnings
 from copy import deepcopy
 from textwrap import dedent
 
@@ -20,6 +19,7 @@
     observe_compat,
     validate,
 )
+from traitlets.utils import warnings
 from traitlets.utils.text import indent, wrap_paragraphs
 
 from .loader import Config, DeferredConfig, LazyConfigValue, _is_section_key
@@ -184,7 +184,10 @@ def _load_config(self, cfg, section_names=None, traits=None):
                     if isinstance(self, LoggingConfigurable):
                         warn = self.log.warning
                     else:
-                        warn = lambda msg: warnings.warn(msg, stacklevel=9)  # noqa[E371]
+
+                        def warn(msg):
+                            return warnings.warn(msg, UserWarning, stacklevel=9)
+
                     matches = get_close_matches(name, traits)
                     msg = "Config option `{option}` not recognized by `{klass}`.".format(
                         option=name, klass=self.__class__.__name__
@@ -452,7 +455,9 @@ def _validate_log(self, proposal):
             # warn about unsupported type, but be lenient to allow for duck typing
             warnings.warn(
                 f"{self.__class__.__name__}.log should be a Logger or LoggerAdapter,"
-                f" got {proposal.value}."
+                f" got {proposal.value}.",
+                UserWarning,
+                stacklevel=2,
             )
         return proposal.value
 

diff --git a/traitlets/config/loader.py b/traitlets/config/loader.py
@@ -11,11 +11,10 @@
 import re
 import sys
 import typing as t
-import warnings
 
 from traitlets.traitlets import Any, Container, Dict, HasTraits, List, Undefined
 
-from ..utils import cast_unicode, filefind
+from ..utils import cast_unicode, filefind, warnings
 
 # -----------------------------------------------------------------------------
 # Exceptions

diff --git a/traitlets/config/tests/test_configurable.py b/traitlets/config/tests/test_configurable.py
@@ -22,9 +22,9 @@
     List,
     Set,
     Unicode,
-    _deprecations_shown,
     validate,
 )
+from traitlets.utils.warnings import _deprecations_shown
 
 from ...tests._warnings import expected_warnings
 

diff --git a/traitlets/traitlets.py b/traitlets/traitlets.py
@@ -48,13 +48,13 @@
 import types
 import typing as t
 from ast import literal_eval
-from warnings import warn, warn_explicit
 
 from .utils.bunch import Bunch
 from .utils.descriptions import add_article, class_of, describe, repr_type
 from .utils.getargspec import getargspec
 from .utils.importstring import import_item
 from .utils.sentinel import Sentinel
+from .utils.warnings import deprecated_method, should_warn, warn
 
 SequenceTypes = (list, tuple, set, frozenset)
 
@@ -161,61 +161,11 @@ class TraitError(Exception):
 # Utilities
 # -----------------------------------------------------------------------------
 
-_name_re = re.compile(r"[a-zA-Z_][a-zA-Z0-9_]*$")
-
 
 def isidentifier(s):
     return s.isidentifier()
 
 
-_deprecations_shown = set()
-
-
-def _should_warn(key):
-    """Add our own checks for too many deprecation warnings.
-
-    Limit to once per package.
-    """
-    env_flag = os.environ.get("TRAITLETS_ALL_DEPRECATIONS")
-    if env_flag and env_flag != "0":
-        return True
-
-    if key not in _deprecations_shown:
-        _deprecations_shown.add(key)
-        return True
-    else:
-        return False
-
-
-def _deprecated_method(method, cls, method_name, msg):
-    """Show deprecation warning about a magic method definition.
-
-    Uses warn_explicit to bind warning to method definition instead of triggering code,
-    which isn't relevant.
-    """
-    warn_msg = "{classname}.{method_name} is deprecated in traitlets 4.1: {msg}".format(
-        classname=cls.__name__, method_name=method_name, msg=msg
-    )
-
-    for parent in inspect.getmro(cls):
-        if method_name in parent.__dict__:
-            cls = parent
-            break
-    # limit deprecation messages to once per package
-    package_name = cls.__module__.split(".", 1)[0]
-    key = (package_name, msg)
-    if not _should_warn(key):
-        return
-    try:
-        fname = inspect.getsourcefile(method) or "<unknown>"
-        lineno = inspect.getsourcelines(method)[1] or 0
-    except (OSError, TypeError) as e:
-        # Failed to inspect for some reason
-        warn(warn_msg + ("\n(inspection failed) %s" % e), DeprecationWarning)
-    else:
-        warn_explicit(warn_msg, DeprecationWarning, fname, lineno)
-
-
 def _safe_literal_eval(s):
     """Safely evaluate an expression
 
@@ -578,7 +528,7 @@ def __init__(
             mod = f.f_globals.get("__name__") or ""
             pkg = mod.split(".", 1)[0]
             key = ("metadata-tag", pkg, *sorted(kwargs))
-            if _should_warn(key):
+            if should_warn(key):
                 warn(
                     "metadata {} was set from the constructor. "
                     "With traitlets 4.1, metadata should be set using the .tag() method, "
@@ -751,7 +701,7 @@ def _cross_validate(self, obj, value):
         elif hasattr(obj, "_%s_validate" % self.name):
             meth_name = "_%s_validate" % self.name
             cross_validate = getattr(obj, meth_name)
-            _deprecated_method(
+            deprecated_method(
                 cross_validate,
                 obj.__class__,
                 meth_name,
@@ -1139,6 +1089,7 @@ def compatible_observer(self, change_or_name, old=Undefined, new=Undefined):
                 "A parent of %s._%s_changed has adopted the new (traitlets 4.1) @observe(change) API"
                 % (clsname, change_or_name),
                 DeprecationWarning,
+                stacklevel=2,
             )
             change = Bunch(
                 type="change",
@@ -1544,7 +1495,7 @@ def _notify_observers(self, event):
         if event['type'] == "change" and hasattr(self, magic_name):
             class_value = getattr(self.__class__, magic_name)
             if not isinstance(class_value, ObserveHandler):
-                _deprecated_method(
+                deprecated_method(
                     class_value,
                     self.__class__,
                     magic_name,
@@ -1724,7 +1675,7 @@ def _register_validator(self, handler, names):
             if hasattr(self, magic_name):
                 class_value = getattr(self.__class__, magic_name)
                 if not isinstance(class_value, ValidateHandler):
-                    _deprecated_method(
+                    deprecated_method(
                         class_value,
                         self.__class__,
                         magic_name,
@@ -2511,7 +2462,8 @@ def from_string(self, s):
                     warn(
                         "Supporting extra quotes around Bytes is deprecated in traitlets 5.0. "
                         "Use {!r} instead of {!r}.".format(s, old_s),
-                        FutureWarning,
+                        DeprecationWarning,
+                        stacklevel=2,
                     )
                     break
         return s.encode("utf8")
@@ -2562,7 +2514,8 @@ def from_string(self, s):
                         "You can use {!r} instead of {!r} if you require traitlets >=5.".format(
                             s, old_s
                         ),
-                        FutureWarning,
+                        DeprecationWarning,
+                        stacklevel=2,
                     )
         return s
 
@@ -2946,7 +2899,8 @@ def from_string_list(self, s_list):
                     "You can pass `--{0} item` ... multiple times to add items to a list.".format(
                         clsname + self.name, r
                     ),
-                    FutureWarning,
+                    DeprecationWarning,
+                    stacklevel=2,
                 )
                 return self.klass(literal_eval(r))  # type:ignore[operator]
         sig = inspect.signature(self.item_from_string)
@@ -3454,7 +3408,8 @@ def from_string_list(self, s_list):
                     self.name,
                     s_list[0],
                 ),
-                FutureWarning,
+                DeprecationWarning,
+                stacklevel=2,
             )
 
             return literal_eval(s_list[0])

diff --git a/traitlets/utils/warnings.py b/traitlets/utils/warnings.py
@@ -0,0 +1,63 @@
+import inspect
+import os
+import warnings
+
+
+def warn(msg, category, *, stacklevel, source=None):
+    """Like warnings.warn(), but category and stacklevel are required.
+
+    You pretty much never want the default stacklevel of 1, so this helps
+    encourage setting it explicitly."""
+    return warnings.warn(msg, category=category, stacklevel=stacklevel, source=source)
+
+
+def deprecated_method(method, cls, method_name, msg):
+    """Show deprecation warning about a magic method definition.
+
+    Uses warn_explicit to bind warning to method definition instead of triggering code,
+    which isn't relevant.
+    """
+    warn_msg = "{classname}.{method_name} is deprecated in traitlets 4.1: {msg}".format(
+        classname=cls.__name__, method_name=method_name, msg=msg
+    )
+
+    for parent in inspect.getmro(cls):
+        if method_name in parent.__dict__:
+            cls = parent
+            break
+    # limit deprecation messages to once per package
+    package_name = cls.__module__.split(".", 1)[0]
+    key = (package_name, msg)
+    if not should_warn(key):
+        return
+    try:
+        fname = inspect.getsourcefile(method) or "<unknown>"
+        lineno = inspect.getsourcelines(method)[1] or 0
+    except (OSError, TypeError) as e:
+        # Failed to inspect for some reason
+        warn(
+            warn_msg + ("\n(inspection failed) %s" % e),
+            DeprecationWarning,
+            stacklevel=2,
+        )
+    else:
+        warnings.warn_explicit(warn_msg, DeprecationWarning, fname, lineno)
+
+
+_deprecations_shown = set()
+
+
+def should_warn(key):
+    """Add our own checks for too many deprecation warnings.
+
+    Limit to once per package.
+    """
+    env_flag = os.environ.get("TRAITLETS_ALL_DEPRECATIONS")
+    if env_flag and env_flag != "0":
+        return True
+
+    if key not in _deprecations_shown:
+        _deprecations_shown.add(key)
+        return True
+    else:
+        return False