Skip to content

Fix compatibility with pygfx - #130

Merged
Korijn merged 3 commits into
masterfrom
fix/pygfx-compatibility
Sep 3, 2025
Merged

Fix compatibility with pygfx#130
Korijn merged 3 commits into
masterfrom
fix/pygfx-compatibility

Conversation

@berendkleinhaneveld

Copy link
Copy Markdown
Collaborator

This change prevents reactivity on custom dict subclasses and instead will check that only 'real' (or pure) dicts are converted into reactive proxies.
The problem is that pygfx's geometry is a dict subclass and turning it into a reactive proxy will stop it from being recognized as a Geometry type, so pygfx will not accept it when you try to set the geometry attribute on a Mesh.
I could check for pygfx specifically, but it seems to me that more problems are to be expected in the future if observ tries to proxy custom dict subclasses.
I'm thinking to update list and set proxy as well.

A workaround in Collagraph is to check for the __target__ attribute when setting the attribute, but I guess that this change also prevents any computational overhead from creating unnecessary proxies of dict subclasses.

This change prevents reactivity on custom dict subclasses and
instead will check that only real dicts are converted into
reactive proxies.
The problem is that pygfx's geometry is a dict subclass and
turning it into a reactive proxy will stop it from being recognized
as a Geometry type, so pygfx will not accept it when you try to
set the geometry attribute on a Mesh.
I could check for pygfx specifically, but it seems to me that
more problems are to be expected in the future if observ tries
to proxy custom dict subclasses.
I'm thinking to update list and set proxy as well.
Comment thread uv.lock Outdated
@Korijn
Korijn merged commit d414db0 into master Sep 3, 2025
8 checks passed
@Korijn
Korijn deleted the fix/pygfx-compatibility branch September 3, 2025 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants