Merge pull request #3569 from jasongrout/stacklevel

Calculate correct stack level for deprecation warnings to show them to the user

Author: ibdafna

python/ipywidgets/ipywidgets/widgets/tests/test_utils.py

@@ -0,0 +1,72 @@
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+import inspect
+import pytest
+
+from ..utils import deprecation
+from .utils import call_method
+
+CALL_PATH = inspect.getfile(call_method)
+
+def test_deprecation():
+    caller_path = inspect.stack(context=0)[1].filename
+    with pytest.deprecated_call() as record:
+        deprecation('Deprecated call')
+    # Make sure the deprecation pointed to the external function calling this test function
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+
+    with pytest.deprecated_call() as record:
+        deprecation('Deprecated call', ['ipywidgets/widgets/tests'])
+    # Make sure the deprecation pointed to the external function calling this test function
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+
+    with pytest.deprecated_call() as record:
+        deprecation('Deprecated call', 'ipywidgets/widgets/tests')
+    # Make sure the deprecation pointed to the external function calling this test function
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+
+    with pytest.deprecated_call() as record:
+        deprecation('Deprecated call', [])
+    # Make sure the deprecation pointed to *this* file
+    assert len(record) == 1
+    assert record[0].filename == __file__
+
+def test_deprecation_indirect():
+    # If the line that calls "deprecation" is not internal, it is considered the source:
+    with pytest.warns(DeprecationWarning) as record:
+        call_method(deprecation, "test message", [])
+    assert len(record) == 1
+    assert record[0].filename == CALL_PATH
+
+def test_deprecation_indirect_internal():
+    # If the line that calls "deprecation" is internal, it is not considered the source:
+    with pytest.warns(DeprecationWarning) as record:
+        call_method(deprecation, "test message", [CALL_PATH])
+    assert len(record) == 1
+    assert record[0].filename == __file__
+
+def test_deprecation_nested1():
+    def level1():
+        deprecation("test message", [])
+
+    with pytest.warns(DeprecationWarning) as record:
+        call_method(level1)
+
+    assert len(record) == 1
+    assert record[0].filename == __file__
+
+def test_deprecation_nested2():
+    def level2():
+        deprecation("test message", [])
+    def level1():
+        level2()
+
+    with pytest.warns(DeprecationWarning) as record:
+        call_method(level1)
+
+    assert len(record) == 1
+    assert record[0].filename == __file__

python/ipywidgets/ipywidgets/widgets/tests/test_widget.py

@@ -6,12 +6,13 @@
 from IPython.core.interactiveshell import InteractiveShell
 from IPython.display import display
 from IPython.utils.capture import capture_output
+import inspect
+import pytest
 
 from .. import widget
 from ..widget import Widget
 from ..widget_button import Button
 
-
 def test_no_widget_view():
     # ensure IPython shell is instantiated
     # otherwise display() just calls print
@@ -51,7 +52,7 @@ def test_close_all():
     widgets = [Button() for i in range(10)]
 
     assert len(widget._instances) > 0, "expect active widgets"
-
+    assert widget._instances[widgets[0].model_id] is widgets[0]
     # close all the widgets
     Widget.close_all()
 
@@ -60,12 +61,16 @@ def test_close_all():
 
 def test_compatibility():
     button = Button()
-    assert button in widget.Widget.widgets.values()
-    assert widget._instances is widget.Widget.widgets
-    assert widget._instances is widget.Widget._active_widgets
-    Widget.close_all()
-    assert not widget.Widget.widgets
-    assert not widget.Widget._active_widgets
-
-    assert widget.Widget.widget_types is widget._registry
-    assert widget.Widget._widget_types is widget._registry
+    assert widget._instances[button.model_id] is button
+    with pytest.deprecated_call() as record:
+        assert widget._instances is widget.Widget.widgets
+        assert widget._instances is widget.Widget._active_widgets
+        assert widget._registry is widget.Widget.widget_types
+        assert widget._registry is widget.Widget._widget_types
+
+        Widget.close_all()
+        assert not widget.Widget.widgets
+        assert not widget.Widget._active_widgets
+    caller_path = inspect.stack(context=0)[1].filename
+    assert all(x.filename == caller_path for x in record)
+    assert len(record) == 6

python/ipywidgets/ipywidgets/widgets/tests/test_widget_button.py

@@ -0,0 +1,12 @@
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+import inspect
+import pytest
+from ipywidgets import Button
+
+def test_deprecation_fa_icons():
+    with pytest.deprecated_call() as record:
+        Button(icon='fa-home')
+    assert len(record) == 1
+    assert record[0].filename == inspect.stack(context=0)[1].filename

python/ipywidgets/ipywidgets/widgets/tests/test_widget_string.py

@@ -1,8 +1,10 @@
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
 
-from ..widget_string import Combobox
+import inspect
+import pytest
 
+from ..widget_string import Combobox, Text
 
 def test_combobox_creation_blank():
     w = Combobox()
@@ -32,3 +34,32 @@ def test_combobox_creation_kwargs():
             "Vanilla",
         )
     assert w.ensure_option == True
+
+def test_tooltip_deprecation():
+    caller_path = inspect.stack(context=0)[1].filename
+    with pytest.deprecated_call() as record:
+        w = Text(description_tooltip="testing")
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+
+    with pytest.deprecated_call() as record:
+        w.description_tooltip
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+
+    with pytest.deprecated_call() as record:
+        w.description_tooltip == "testing"
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+
+    with pytest.deprecated_call() as record:
+        w.description_tooltip = "second value"
+    assert len(record) == 1
+    assert record[0].filename == caller_path
+    assert w.tooltip == "second value"
+
+def test_on_submit_deprecation():
+    with pytest.deprecated_call() as record:
+        Text().on_submit(lambda *args: ...)
+    assert len(record) == 1
+    assert record[0].filename == inspect.stack(context=0)[1].filename

python/ipywidgets/ipywidgets/widgets/tests/utils.py

@@ -48,3 +48,6 @@ def setup():
 
 def teardown():
     teardown_test_comm()
+
+def call_method(method, *args, **kwargs):
+    method(*args, **kwargs)

python/ipywidgets/ipywidgets/widgets/utils.py

@@ -0,0 +1,64 @@
+# Copyright (c) Jupyter Development Team.
+# Distributed under the terms of the Modified BSD License.
+
+from pathlib import Path
+import sys
+import inspect
+import warnings
+
+def _get_frame(level):
+    """Get the frame at the given stack level."""
+    # sys._getframe is much faster than inspect.stack, but isn't guaranteed to
+    # exist in all python implementations, so we fall back to inspect.stack()
+
+    # We need to add one to level to account for this get_frame call.
+    if hasattr(sys, '_getframe'):
+        frame = sys._getframe(level+1)
+    else:
+        frame = inspect.stack(context=0)[level+1].frame
+    return frame
+
+
+# This function is from https://github.com/python/cpython/issues/67998
+# (https://bugs.python.org/file39550/deprecated_module_stacklevel.diff) and
+# calculates the appropriate stacklevel for deprecations to target the
+# deprecation for the caller, no matter how many internal stack frames we have
+# added in the process. For example, with the deprecation warning in the
+# __init__ below, the appropriate stacklevel will change depending on how deep
+# the inheritance hierarchy is.
+def _external_stacklevel(internal):
+    """Find the stacklevel of the first frame that doesn't contain any of the given internal strings
+
+    The depth will be 1 at minimum in order to start checking at the caller of
+    the function that called this utility method.
+    """
+    # Get the level of my caller's caller
+    level = 2
+    frame = _get_frame(level)
+
+    # Normalize the path separators:
+    normalized_internal = [str(Path(s)) for s in internal]
+
+    # climb the stack frames while we see internal frames
+    while frame and any(s in str(Path(frame.f_code.co_filename)) for s in normalized_internal):
+        level +=1
+        frame = frame.f_back
+
+    # Return the stack level from the perspective of whoever called us (i.e., one level up)
+    return level-1
+
+def deprecation(message, internal='ipywidgets/widgets/'):
+    """Generate a deprecation warning targeting the first frame that is not 'internal'
+    
+    internal is a string or list of strings, which if they appear in filenames in the
+    frames, the frames will be considered internal. Changing this can be useful if, for examnple,
+    we know that ipywidgets is calling out to traitlets internally.
+    """
+    if isinstance(internal, str):
+        internal = [internal]
+
+    # stack level of the first external frame from here
+    stacklevel = _external_stacklevel(internal)
+
+    # The call to .warn adds one frame, so bump the stacklevel up by one
+    warnings.warn(message, DeprecationWarning, stacklevel=stacklevel+1)

python/ipywidgets/ipywidgets/widgets/widget.py

@@ -9,7 +9,6 @@
 import typing
 from contextlib import contextmanager
 from collections.abc import Iterable
-import warnings
 from IPython import get_ipython
 from ipykernel.comm import Comm
 from traitlets import (
@@ -19,8 +18,13 @@
 
 from base64 import standard_b64encode
 
+from .utils import deprecation, _get_frame
+
 from .._version import __protocol_version__, __control_protocol_version__, __jupyter_widgets_base_version__
 
+import inspect
+TRAITLETS_FILE = inspect.getfile(HasTraits)
+
 # Based on jupyter_core.paths.envset
 def envset(name, default):
     """Return True if the given environment variable is turned on, otherwise False
@@ -302,22 +306,42 @@ class Widget(LoggingHasTraits):
 
     @_staticproperty
     def widgets():
-        warnings.warn("Widget.widgets is deprecated.", DeprecationWarning)
+        # Because this is a static attribute, it will be accessed when initializing this class. In that case, since a user
+        # did not explicitly try to use this attribute, we do not want to throw a deprecation warning.
+        # So we check if the thing calling this static property is one of the known initialization functions in traitlets.
+        frame = _get_frame(2)
+        if not (frame.f_code.co_filename == TRAITLETS_FILE and (frame.f_code.co_name in ('getmembers', 'setup_instance'))):
+            deprecation("Widget.widgets is deprecated.")
         return _instances
 
     @_staticproperty
     def _active_widgets():
-        warnings.warn("Widget._active_widgets is deprecated.", DeprecationWarning)
+        # Because this is a static attribute, it will be accessed when initializing this class. In that case, since a user
+        # did not explicitly try to use this attribute, we do not want to throw a deprecation warning.
+        # So we check if the thing calling this static property is one of the known initialization functions in traitlets.
+        frame = _get_frame(2)
+        if not (frame.f_code.co_filename == TRAITLETS_FILE and (frame.f_code.co_name in ('getmembers', 'setup_instance'))):
+            deprecation("Widget._active_widgets is deprecated.")
         return _instances
 
     @_staticproperty
     def _widget_types():
-        warnings.warn("Widget._widget_types is deprecated.", DeprecationWarning)
+        # Because this is a static attribute, it will be accessed when initializing this class. In that case, since a user
+        # did not explicitly try to use this attribute, we do not want to throw a deprecation warning.
+        # So we check if the thing calling this static property is one of the known initialization functions in traitlets.
+        frame = _get_frame(2)
+        if not (frame.f_code.co_filename == TRAITLETS_FILE and (frame.f_code.co_name in ('getmembers', 'setup_instance'))):
+            deprecation("Widget._widget_types is deprecated.")
         return _registry
 
     @_staticproperty
     def widget_types():
-        warnings.warn("Widget.widget_types is deprecated.", DeprecationWarning)
+        # Because this is a static attribute, it will be accessed when initializing this class. In that case, since a user
+        # did not explicitly try to use this attribute, we do not want to throw a deprecation warning.
+        # So we check if the thing calling this static property is one of the known initialization functions in traitlets.
+        frame = _get_frame(2)
+        if not (frame.f_code.co_filename == TRAITLETS_FILE and (frame.f_code.co_name in ('getmembers', 'setup_instance'))):
+            deprecation("Widget.widget_types is deprecated.")
         return _registry
 
     @classmethod

python/ipywidgets/ipywidgets/widgets/widget_button.py

@@ -7,14 +7,14 @@
 click events on the button and trigger backend code when the clicks are fired.
 """
 
+from .utils import deprecation
 from .domwidget import DOMWidget
 from .widget import CallbackDispatcher, register, widget_serialization
 from .widget_core import CoreWidget
 from .widget_style import Style
 from .trait_types import Color, InstanceDict
 
 from traitlets import Unicode, Bool, CaselessStrEnum, Instance, validate, default
-import warnings
 
 
 @register
@@ -70,8 +70,9 @@ def _validate_icon(self, proposal):
         """Strip 'fa-' if necessary'"""
         value = proposal['value']
         if 'fa-' in value:
-            warnings.warn("icons names no longer need 'fa-', "
-            "just use the class names themselves (for example, 'gear spin' instead of 'fa-gear fa-spin')", DeprecationWarning)
+            deprecation("icons names no longer need 'fa-', "
+            "just use the class names themselves (for example, 'gear spin' instead of 'fa-gear fa-spin')",
+            internal=['ipywidgets/widgets/', 'traitlets/traitlets.py', '/contextlib.py'])
             value = value.replace('fa-', '')
         return value
 

python/ipywidgets/ipywidgets/widgets/widget_description.py

@@ -9,6 +9,8 @@
 from .widget_style import Style
 from .widget_core import CoreWidget
 from .domwidget import DOMWidget
+from .utils import deprecation
+
 import warnings
 
 @register
@@ -27,7 +29,7 @@ class DescriptionWidget(DOMWidget, CoreWidget):
 
     def __init__(self, *args, **kwargs):
         if 'description_tooltip' in kwargs:
-            warnings.warn("the description_tooltip argument is deprecated, use tooltip instead", DeprecationWarning)
+            deprecation("the description_tooltip argument is deprecated, use tooltip instead")
             kwargs.setdefault('tooltip', kwargs['description_tooltip'])
             del kwargs['description_tooltip']
         super().__init__(*args, **kwargs)
@@ -47,10 +49,10 @@ def description_tooltip(self):
         .. deprecated :: 8.0.0
            Use tooltip attribute instead.
         """
-        warnings.warn(".description_tooltip is deprecated, use .tooltip instead", DeprecationWarning)
+        deprecation(".description_tooltip is deprecated, use .tooltip instead")
         return self.tooltip
 
     @description_tooltip.setter
     def description_tooltip(self, tooltip):
-        warnings.warn(".description_tooltip is deprecated, use .tooltip instead", DeprecationWarning)
+        deprecation(".description_tooltip is deprecated, use .tooltip instead")
         self.tooltip = tooltip