Open manzt opened 1 month ago
Latest commit: 1ec7789bf11ee05e72f9aec8bceaf31d410be5cf
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
For example, with this PR, widget authors could extend directly from traitlets.HasTraits
and work with regular ipywidgets:
import traitlets
import ipywidgets
from anywidget._descriptor import MimeBundleDescriptor
esm = """
export function render({ model, el }) {
let count = () => model.get("value");
let btn = document.createElement("button");
btn.classList.add("counter-button");
btn.innerHTML = `count is ${count()}`;
btn.addEventListener("click", () => {
model.set("value", count() + 1);
model.save_changes();
});
model.on("change:value", () => {
btn.innerHTML = `count is ${count()}`;
});
el.appendChild(btn);
}
"""
css = """
.counter-button {
background-color: rgba(148, 163, 184, 0.1);
border-radius: 10px;
padding: 0.5em 1em;
font-size: 2em;
}
"""
class Counter(traitlets.HasTraits):
_repr_mimebundle_ = MimeBundleDescriptor(_esm=esm, _css=css)
value = traitlets.Int(0).tag(sync=True)
counter = Counter()
ipywidgets.VBox([counter, ipywidgets.IntSlider()])
@tlambert03, I'm really flummoxed here. I believe it's crucial to maintain compatibility with these top-level APIs from ipywidgets, but the deep connection with ipywidgets.Widget is really holding us back.
I think this is the minimal "patching" that is necessary, but it's definitely a hack and I don't love that it mutates ipywidgets. I just worry it's going to take a long time to upstream something like this.
haven't fully looked through your _patch_ipywidgets
solution yet... but, just a question: if you could somehow "magically" make some anywidget object pass the isinstance(x, Widget)
check, would that solve the problem?
(he asks, almost as if to suggest such a thing is possible...)
if you could somehow "magically" make some anywidget object pass the isinstance(x, Widget) check, would that solve the problem?
Yes... that would solve everything I think (if we also exposed a model_id
on that object on our side). However, trying to look into how to do that I couldn't figure it out. Or just came across stack overflow answers that seemed to suggest it's really tricky.
hmmm, yeah, i was hoping i could be sneaky with __instancecheck__
https://docs.python.org/3/reference/datamodel.html#customizing-instance-and-subclass-checks ... but running into probably the same difficulties you did. It's easy to make anything else look like an instance of something you own, but hard to make something you own look like an instance of anything else
here's another crazy black magic idea:
In [8]: import ipywidgets
In [9]: class MyThing:
...: @property
...: def __class__(self):
...: return ipywidgets.Widget
...:
In [10]: isinstance(MyThing(), ipywidgets.Widget)
Out[10]: True
so, taking that one step further, if you did a little stack inspection inside of that __class__
method, and checked "who's asking" ... and lied only to ipywidgets, it could work
import inspect
from ipywidgets.widgets.widget import _widget_to_json
import ipywidgets
class LieToIpyWidgets:
def __get__(self, obj, objtype=None):
if obj is None:
return self
stack = inspect.stack()
for frame in stack:
if "ipywidgets" in frame.filename: # this could be better of course
return ipywidgets.Widget
return objtype
class Thing:
__class__ = LieToIpyWidgets()
model_id = "1234"
thing = Thing()
print(isinstance(thing, ipywidgets.Widget))
print(_widget_to_json(thing, {}))
➜ python x.py
False
IPY_MODEL_1234
(you could even setattr(owner, '__class__', LieToIpyWidgets())
inside of MimeBundleDescriptor.__set_name__
)
not saying it's much better than what you're currently doing ... but I do definitely hesitate to monkey patch other peoples libraries, and they would be justified in being annoyed by that
not saying it's much better than what you're currently doing ... but I do definitely hesitate to monkey patch other peoples libraries, and they would be justified in being annoyed by that
Totally. I like your solution a lot better. I'm going to go in that direction. Thanks so much for the experimentation and thoughts. As always, they are very creative and have the right amount of magic. I always learn something :) I'm not sure I would have come to a similar alternative.
Summary
ipywidgets performs an
isinstance(x, Widget)
check with their container classes (i.e.,Box
,Link
). This restricts the use of only classes that derive fromipywidgets.Widget
, which is a very large class with many methods.This is problematic because it ties the entire widgets ecosystem (and anywidget) to a large, monolithic class. Widget authors inherit from this class, and now their top-level APIs for end users have a set of methods that should be considered private to end users but are useful for widget developers.
This makes it difficult to innovate on Python-side APIs, as developers are forced to inherit from this large class even if they only need basic widget functionality.
In anywidget, it really holds us back from iterating on the Python side of thing because we don't want to break compatability with ipywidgets but we need other APIs (#579, #628).
Solution
This PR patches ipywidgets to allow widgets composed from non-
Widget
base classes. The patch performs flexible serialization and deserialization by checking for the model_id attribute instead of strict instance checks.We should probably come up with a protocol for getting the
model_id
attribute, making this even less tied to anywidget/ipywidgets, but for now, this is a good start.For example, you can now implement a very simple widget with our descriptor API: