From ca3d0cb0964e7ad333515399eb1600bcbe03c248 Mon Sep 17 00:00:00 2001 From: Alan Fleming <> Date: Tue, 5 Nov 2024 08:16:29 +1100 Subject: [PATCH] Change _instances from 'global' variable to class variable. --- python/ipywidgets/ipywidgets/embed.py | 6 +- .../ipywidgets/ipywidgets/tests/test_embed.py | 4 +- .../ipywidgets/widgets/tests/test_widget.py | 39 ++------ .../widgets/tests/test_widget_box.py | 2 - .../ipywidgets/ipywidgets/widgets/widget.py | 96 +++++-------------- python/ipywidgets/setup.cfg | 2 + 6 files changed, 41 insertions(+), 108 deletions(-) diff --git a/python/ipywidgets/ipywidgets/embed.py b/python/ipywidgets/ipywidgets/embed.py index ca58880092..e9bffbfd65 100644 --- a/python/ipywidgets/ipywidgets/embed.py +++ b/python/ipywidgets/ipywidgets/embed.py @@ -12,7 +12,7 @@ import json import re -from .widgets import Widget, DOMWidget, widget as widget_module +from .widgets import Widget, DOMWidget from .widgets.widget_link import Link from .widgets.docutils import doc_subst from ._version import __html_manager_version__ @@ -129,7 +129,7 @@ def _get_recursive_state(widget, store=None, drop_defaults=False): def add_resolved_links(store, drop_defaults): """Adds the state of any link models between two models in store""" - for widget_id, widget in widget_module._instances.items(): # go over all widgets + for widget_id, widget in Widget._instances.items(): # go over all widgets if isinstance(widget, Link) and widget_id not in store: if widget.source[0].model_id in store and widget.target[0].model_id in store: store[widget.model_id] = widget._get_embed_state(drop_defaults=drop_defaults) @@ -207,7 +207,7 @@ def embed_data(views, drop_defaults=True, state=None): view_specs: a list of widget view specs """ if views is None: - views = [w for w in widget_module._instances.values() if isinstance(w, DOMWidget)] + views = [w for w in Widget._instances.values() if isinstance(w, DOMWidget)] else: try: views[0] diff --git a/python/ipywidgets/ipywidgets/tests/test_embed.py b/python/ipywidgets/ipywidgets/tests/test_embed.py index a295442455..c5e6b97aa1 100644 --- a/python/ipywidgets/ipywidgets/tests/test_embed.py +++ b/python/ipywidgets/ipywidgets/tests/test_embed.py @@ -9,7 +9,7 @@ import traitlets -from ..widgets import IntSlider, IntText, Text, Widget, jslink, HBox, widget_serialization, widget as widget_module +from ..widgets import IntSlider, IntText, Text, Widget, jslink, HBox, widget_serialization from ..embed import embed_data, embed_snippet, embed_minimal_html, dependency_state @@ -29,7 +29,7 @@ class CaseWidget(Widget): class TestEmbed: def teardown(self): - for w in tuple(widget_module._instances.values()): + for w in tuple(Widget._instances.values()): w.close() def test_embed_data_simple(self): diff --git a/python/ipywidgets/ipywidgets/widgets/tests/test_widget.py b/python/ipywidgets/ipywidgets/widgets/tests/test_widget.py index 34fd9402a2..dbd949dd10 100644 --- a/python/ipywidgets/ipywidgets/widgets/tests/test_widget.py +++ b/python/ipywidgets/ipywidgets/widgets/tests/test_widget.py @@ -5,7 +5,6 @@ import copy import gc -import inspect import weakref import pytest @@ -15,7 +14,6 @@ import ipywidgets as ipw -from .. import widget from ..widget import Widget from ..widget_button import Button @@ -62,29 +60,12 @@ def test_close_all(): # create a couple of widgets 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] + 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() - assert len(widget._instances) == 0, "active widgets should be cleared" - - -def test_compatibility(): - button = Button() - 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 + assert len(Widget._instances) == 0, "active widgets should be cleared" def test_widget_copy(): @@ -98,12 +79,12 @@ def test_widget_copy(): def test_widget_open(): button = Button() model_id = button.model_id - assert model_id in widget._instances + assert model_id in Widget._instances spec = button.get_view_spec() assert list(spec) == ["version_major", "version_minor", "model_id"] assert spec["model_id"] button.close() - assert model_id not in widget._instances + assert model_id not in Widget._instances with pytest.raises(RuntimeError, match="Widget is closed"): button.open() with pytest.raises(RuntimeError, match="Widget is closed"): @@ -225,7 +206,7 @@ def my_click(self, b): b = TestButton(description="button") weakref.finalize(b, on_delete) b_ref = weakref.ref(b) - assert b in widget._instances.values() + assert b in Widget._instances.values() b.on_click(b.my_click) b.on_click(lambda x: setattr(x, "clicked", True)) @@ -235,11 +216,11 @@ def my_click(self, b): if weakref_enabled: ipw.enable_weakreference() - assert b in widget._instances.values(), "Instances not transferred" + assert b in Widget._instances.values(), "Instances not transferred" ipw.disable_weakreference() - assert b in widget._instances.values(), "Instances not transferred" + assert b in Widget._instances.values(), "Instances not transferred" ipw.enable_weakreference() - assert b in widget._instances.values(), "Instances not transferred" + assert b in Widget._instances.values(), "Instances not transferred" b.click() assert click_count == 2 @@ -251,7 +232,7 @@ def my_click(self, b): assert deleted else: assert not deleted - assert b_ref() in widget._instances.values() + assert b_ref() in Widget._instances.values() b_ref().close() gc.collect() assert deleted, "Closing should remove the last strong reference." diff --git a/python/ipywidgets/ipywidgets/widgets/tests/test_widget_box.py b/python/ipywidgets/ipywidgets/widgets/tests/test_widget_box.py index 5d50324d08..021e3f4c78 100644 --- a/python/ipywidgets/ipywidgets/widgets/tests/test_widget_box.py +++ b/python/ipywidgets/ipywidgets/widgets/tests/test_widget_box.py @@ -57,7 +57,6 @@ def test_box_validate_mode(): def test_box_gc(): - widgets.VBox._active_widgets widgets.enable_weakreference() # Test Box gc collected and children lifecycle managed. try: @@ -80,6 +79,5 @@ def on_delete(): del b gc.collect() assert deleted - widgets.VBox._active_widgets finally: widgets.disable_weakreference() diff --git a/python/ipywidgets/ipywidgets/widgets/widget.py b/python/ipywidgets/ipywidgets/widgets/widget.py index 6eda43e809..c95850132b 100644 --- a/python/ipywidgets/ipywidgets/widgets/widget.py +++ b/python/ipywidgets/ipywidgets/widgets/widget.py @@ -19,8 +19,6 @@ 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 @@ -43,29 +41,24 @@ def envset(name, default): JUPYTER_WIDGETS_ECHO = envset('JUPYTER_WIDGETS_ECHO', default=True) # for a discussion on using weak references see: # https://github.com/jupyter-widgets/ipywidgets/issues/1345 -_instances : typing.MutableMapping[str, "Widget"] = {} def enable_weakreference(): - """Use a WeakValueDictionary instead of a standard dictionary to map - `comm_id` to `widget` for every widget instance. + """Use a WeakValueDictionary to store references to Widget instances. - By default widgets are mapped using a standard dictionary. Use this feature - to permit widget garbage collection. + A strong reference must be kept to widgets. """ - global _instances - if not isinstance(_instances, weakref.WeakValueDictionary): - _instances = weakref.WeakValueDictionary(_instances) + if not isinstance(Widget._instances, weakref.WeakValueDictionary): + Widget._instances = weakref.WeakValueDictionary(Widget._instances) def disable_weakreference(): - """Use a Dictionary to map `comm_id` to `widget` for every widget instance. - - Note: this is the default setting and maintains a strong reference to the - the widget preventing automatic garbage collection. If the close method - is called, the widget will remove itself enabling garbage collection. + """Use a standard dictionary to store references to Widget instances (default behavior). + + Note: this is the default setting and maintains a strong reference to the + the widget preventing automatic garbage collection. When the widget is closed + it can be garbage collected. """ - global _instances - if isinstance(_instances, weakref.WeakValueDictionary): - _instances = dict(_instances) + if isinstance(Widget._instances, weakref.WeakValueDictionary): + Widget._instances = dict(Widget._instances) def _widget_to_json(x, obj): if isinstance(x, Widget): @@ -82,8 +75,8 @@ def _json_to_widget(x, obj): return {k: _json_to_widget(v, obj) for k, v in x.items()} elif isinstance(x, (list, tuple)): return [_json_to_widget(v, obj) for v in x] - elif isinstance(x, str) and x.startswith('IPY_MODEL_') and x[10:] in _instances: - return _instances[x[10:]] + elif isinstance(x, str) and x.startswith("IPY_MODEL_") and x[10:] in Widget._instances: + return Widget._instances[x[10:]] else: return x @@ -314,50 +307,12 @@ class Widget(LoggingHasTraits): #------------------------------------------------------------------------- _widget_construction_callback = None _control_comm = None - - @_staticproperty - def widgets(): - # 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', 'setup_class'))): - deprecation("Widget.widgets is deprecated.") - return _instances - - @_staticproperty - def _active_widgets(): - # 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', 'setup_class'))): - deprecation("Widget._active_widgets is deprecated.") - return _instances - - @_staticproperty - def _widget_types(): - # 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', 'setup_class'))): - deprecation("Widget._widget_types is deprecated.") - return _registry - - @_staticproperty - def widget_types(): - # 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', 'setup_class'))): - deprecation("Widget.widget_types is deprecated.") - return _registry + + _instances: typing.ClassVar[typing.MutableMapping[str, "Widget"]] = {} @classmethod def close_all(cls): - for widget in list(_instances.values()): + for widget in list(Widget._instances.values()): widget.close() @staticmethod @@ -399,7 +354,7 @@ def _handle_control_comm_msg(cls, msg): if method == 'request_states': # Send back the full widgets state cls.get_manager_state() - widgets = _instances.values() + widgets = cls._instances.values() full_state = {} drop_defaults = False for widget in widgets: @@ -440,8 +395,8 @@ def handle_comm_opened(comm, msg): _put_buffers(state, data['buffer_paths'], msg['buffers']) widget.set_state(state) - @staticmethod - def get_manager_state(drop_defaults=False, widgets=None): + @classmethod + def get_manager_state(cls, drop_defaults=False, widgets=None): """Returns the full state for a widget manager for embedding :param drop_defaults: when True, it will not include default value @@ -450,7 +405,7 @@ def get_manager_state(drop_defaults=False, widgets=None): """ state = {} if widgets is None: - widgets = _instances.values() + widgets = cls._instances.values() for widget in widgets: state[widget.model_id] = widget._get_embed_state(drop_defaults=drop_defaults) return {'version_major': 2, 'version_minor': 0, 'state': state} @@ -561,14 +516,11 @@ def _comm_changed(self, change): if change['old']: change['old'].on_msg(None) change['old'].close() - # On python shutdown _instances can be None - if isinstance(_instances, dict): - _instances.pop(change['old'].comm_id, None) + self._instances.pop(change['old'].comm_id, None) if change['new']: - if isinstance(_instances, dict): - _instances[change["new"].comm_id] = self - self._model_id = change["new"].comm_id - + self._instances[change["new"].comm_id] = self + self._model_id = change["new"].comm_id + # prevent memory leaks by using a weak reference to self. ref = weakref.ref(self) def _handle_msg(msg): diff --git a/python/ipywidgets/setup.cfg b/python/ipywidgets/setup.cfg index 36fbdf847c..c3fb6a9fda 100644 --- a/python/ipywidgets/setup.cfg +++ b/python/ipywidgets/setup.cfg @@ -23,6 +23,8 @@ classifiers = Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 Programming Language :: Python :: 3.11 + Programming Language :: Python :: 3.12 + Programming Language :: Python :: 3.13 Programming Language :: Python :: 3 :: Only Framework :: Jupyter