Skip to content

Commit

Permalink
Change _instances from 'global' variable to class variable.
Browse files Browse the repository at this point in the history
  • Loading branch information
Alan Fleming committed Nov 4, 2024
1 parent dec5524 commit ca3d0cb
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 108 deletions.
6 changes: 3 additions & 3 deletions python/ipywidgets/ipywidgets/embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions python/ipywidgets/ipywidgets/tests/test_embed.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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):
Expand Down
39 changes: 10 additions & 29 deletions python/ipywidgets/ipywidgets/widgets/tests/test_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

import copy
import gc
import inspect
import weakref

import pytest
Expand All @@ -15,7 +14,6 @@

import ipywidgets as ipw

from .. import widget
from ..widget import Widget
from ..widget_button import Button

Expand Down Expand Up @@ -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():
Expand All @@ -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"):
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand All @@ -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."
Expand Down
2 changes: 0 additions & 2 deletions python/ipywidgets/ipywidgets/widgets/tests/test_widget_box.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -80,6 +79,5 @@ def on_delete():
del b
gc.collect()
assert deleted
widgets.VBox._active_widgets
finally:
widgets.disable_weakreference()
96 changes: 24 additions & 72 deletions python/ipywidgets/ipywidgets/widgets/widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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}
Expand Down Expand Up @@ -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):
Expand Down
2 changes: 2 additions & 0 deletions python/ipywidgets/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit ca3d0cb

Please sign in to comment.