Skip to content

Fix scene-object registration interfering with other COMPAS backends#5

Open
arpastrana wants to merge 2 commits into
compas-dev:mainfrom
arpastrana:main
Open

Fix scene-object registration interfering with other COMPAS backends#5
arpastrana wants to merge 2 commits into
compas-dev:mainfrom
arpastrana:main

Conversation

@arpastrana

Copy link
Copy Markdown
Contributor

Summary

compas_plotter registered its scene objects eagerly at import time. That side effect broke scene-object resolution for every other COMPAS visualization backend (the Viewer and the Notebook) whenever compas_plotter happened to be imported first.

This PR removes the eager registration and instead exposes the plugin the way compas_viewer does, via __all_plugins__, so COMPAS discovers it through the standard lazy collect_all path.

The bug

Note the script never imports compas_plotter.

from compas.datastructures import Mesh
from compas.geometry import NurbsCurve
from compas_viewer import Viewer

mesh = Mesh.from_meshgrid(dx=10, nx=10)
arch = NurbsCurve.from_points([[5, 0, 0], [5, 5, 5], [5, 10, 0]])  # the trigger

viewer = Viewer()
viewer.scene.add(mesh)   # -> SceneObjectNotRegisteredError: ... context: Viewer
viewer.scene.add(arch)
viewer.show()

Root cause

COMPAS lazily initialises its scene-object registry with a one-shot guard in
compas/scene/context.py:

def get_sceneobject_cls(item, **kwargs):
    if not ITEM_SCENEOBJECT:          # only fires while the registry is empty
        register_scene_objects()      # the collect_all pluggable — ALL backends
    ...

The register_scene_objects pluggable uses the collect_all selector, so this single call is what registers every backend (base COMPAS, Viewer, Notebook, Plotter, …). It only runs while ITEM_SCENEOBJECT is still empty.

The failure chain:

  1. NurbsCurve.from_points(...) needs a geometry backend, so COMPAS runs plugin discovery, which imports every installed compas_* package — including compas_plotter.
  2. Importing compas_plotter ran register_scene_objects() eagerly (via from . import scene and the eager call at the end of scene/__init__.py), writing ITEM_SCENEOBJECT["Plotter"] = {...}.
  3. ITEM_SCENEOBJECT is now non-empty, so COMPAS's if not ITEM_SCENEOBJECT guard never fires. The collect_all discovery that would have registered the Viewer and base contexts is skipped.
  4. viewer.scene.add(mesh) can't resolve Mesh in the "Viewer" context and raises SceneObjectNotRegisteredError.

compas_plotter was also missing __all_plugins__, so the PluginManager never imported compas_plotter.scene to find the @plugin-decorated factory the proper way — the eager import was compensating for that, which is exactly what messed up with the guard.

The fix

  • Add __all_plugins__ = ["compas_plotter.scene"] in compas_plotter/__init__.py (mirrors compas_viewer).
  • Remove the from . import scene side-effect import.
  • Remove the eager register_scene_objects() call at the end of compas_plotter/scene/__init__.py.

Registration now happens only through COMPAS's plugin discovery + lazy collect_all, so backends no longer clobber each other's registration.

Verification

Reproduced and fixed against compas 2.15.1, compas_viewer 2.0.2, compas_occ.

  • Before: the MRE raises SceneObjectNotRegisteredError on viewer.scene.add(mesh).
  • After: the MRE adds both objects with no exception; mesh resolves to MeshObject and arch to NurbsCurveObject.
  • compas_plotter standalone still registers and draws (Plotter().add(mesh)
  • New regression test test_import_does_not_populate_registry_eagerly asserts,
    in a subprocess, that importing compas_plotter leaves ITEM_SCENEOBJECT empty and that collect_all then registers both the Plotter and base contexts.
  • Full suite: 16 passed. ruff check / ruff format --check clean.

Eager import-time registration populated COMPAS's global ITEM_SCENEOBJECT, which suppressed the lazy discovery trigger in compas.scene.context and left other backends unregistered. Declare __all_plugins__ so the factory is found through the standard collect_all path instead.
Add test_import_does_not_populate_registry_eagerly, run in a subprocess for a pristine interpreter (ITEM_SCENEOBJECT is a process-global latch). Hoist the module-level imports it shares with existing tests to the top of the file.
@arpastrana

Copy link
Copy Markdown
Contributor Author

I updated the changelog, so not sure why the PR checklist does not pass in CI...

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.

1 participant