From 006a24ef32a9c79f24e9880e5364571bdd61796a Mon Sep 17 00:00:00 2001 From: Gorgious56 Date: Tue, 23 Jun 2026 14:27:29 +0200 Subject: [PATCH] Refactor: rename rel -> subject in disconnect_rel dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "mep-pair-fitting" kind added in the previous commit carries an IfcFlowFitting (the entity whose deletion disconnects the pair), not a relationship entity, in the dispatch slot — but the slot was named ``rel`` across the function signature and every call site. Rename to ``subject`` so the parameter name reflects the uniform intent: "the entity whose teardown effects the disconnect", regardless of whether that's a rel or a fitting. Sweep covers: - core.connection.disconnect_rel signature + body - tool.Connection.find_rels / find_rels_for_element / find_rel docstrings - The cascade-on-delete call site in tool.Geometry.delete_ifc_object - DisconnectElements operator in bim.module.model.wall - All affected test kwargs and AST forward-compat docstring - Error message: "Unknown rel kind" -> "Unknown kind" No behaviour change. Generated with the assistance of an AI coding tool. --- src/bonsai/bonsai/bim/module/model/wall.py | 4 +- src/bonsai/bonsai/core/connection.py | 40 +++++++------- src/bonsai/bonsai/tool/connection.py | 54 ++++++++++--------- src/bonsai/bonsai/tool/geometry.py | 4 +- .../module/model/test_disconnect_elements.py | 6 +-- .../model/test_mep_disconnect_integration.py | 6 +-- src/bonsai/test/core/test_connection.py | 30 +++++------ .../tool/test_connection_forward_compat.py | 6 +-- 8 files changed, 76 insertions(+), 74 deletions(-) diff --git a/src/bonsai/bonsai/bim/module/model/wall.py b/src/bonsai/bonsai/bim/module/model/wall.py index dd054635a7..eaf4c356be 100644 --- a/src/bonsai/bonsai/bim/module/model/wall.py +++ b/src/bonsai/bonsai/bim/module/model/wall.py @@ -400,13 +400,13 @@ class DisconnectElements(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.I ) return path_objs: list[bpy.types.Object] = [] - for rel, kind in rels: + for subject, kind in rels: bonsai.core.connection.disconnect_rel( tool.Ifc, tool.Geometry, tool.Model, tool.Connection, - rel=rel, + subject=subject, kind=kind, elem=elem_a, partner=elem_b, diff --git a/src/bonsai/bonsai/core/connection.py b/src/bonsai/bonsai/core/connection.py index 8b5d80d898..3ea3bd4bef 100644 --- a/src/bonsai/bonsai/core/connection.py +++ b/src/bonsai/bonsai/core/connection.py @@ -22,17 +22,18 @@ Used by both ``bim.disconnect_elements`` (explicit user disconnect) and the connection cascade in ``tool.Geometry.delete_ifc_object`` (implicit -disconnect-on-delete). Each rel kind returned by +disconnect-on-delete). Each kind returned by :py:meth:`bonsai.tool.connection.Connection.find_rels` / :py:meth:`find_rels_for_element` maps to a single arm here, so adding a new -rel kind means extending one dispatch table — both call sites benefit +kind means extending one dispatch table — both call sites benefit automatically and the AST forward-compat guard enforces coverage. -For the ``"mep-pair-fitting"`` kind the ``rel`` slot carries the -``IfcFlowFitting`` itself rather than a relationship entity — the subject -whose removal disconnects the pair. The dispatch treats the slot as the -deletion target and routes through ``geometry.delete_ifc_object`` so the -cascade-on-delete contract still owns port-rel cleanup. +The ``subject`` parameter is the entity whose teardown effects the +disconnect: for ``"path"`` / ``"element"`` / ``"element-top"`` kinds it +carries an ``IfcRel*`` relationship entity (the rel that gets removed); +for ``"mep-pair-fitting"`` it carries an ``IfcFlowFitting`` (the fitting +that gets deleted). The slot is uniform on intent — the dispatch decides +the teardown mechanism by kind. """ from __future__ import annotations @@ -53,14 +54,14 @@ def disconnect_rel( geometry: type[tool.Geometry], model: type[tool.Model], connection: type[tool.Connection], - rel: ifcopenshell.entity_instance, + subject: ifcopenshell.entity_instance, kind: str, elem: ifcopenshell.entity_instance, partner: ifcopenshell.entity_instance, skip_elem_recreate: bool = False, skip_partner_recreate: bool = False, ) -> None: - """Run the post-disconnect cleanup for one rel. + """Run the post-disconnect cleanup for one connection. ``elem`` and ``partner`` are the two endpoints. The ``skip_*_recreate`` flags suppress per-side regenerate / recreate work — used by the @@ -70,7 +71,7 @@ def disconnect_rel( runs on both sides. """ if kind == "path": - bonsai.core.geometry.remove_connection(geometry, connection=rel) + bonsai.core.geometry.remove_connection(geometry, connection=subject) if not skip_elem_recreate: elem_obj = ifc.get_object(elem) if elem_obj is not None: @@ -80,11 +81,11 @@ def disconnect_rel( if partner_obj is not None: model.recreate_wall(partner, partner_obj) elif kind == "element-top": - wall, _slab = connection.orient_element_top(rel, elem, partner) + wall, _slab = connection.orient_element_top(subject, elem, partner) ifc.run( "geometry.disconnect_element", - relating_element=rel.RelatingElement, - related_element=rel.RelatedElement, + relating_element=subject.RelatingElement, + related_element=subject.RelatedElement, ) # Skip the wall-side regenerate when the wall is itself being deleted — # either it's the elem of this cascade pass, or it's the partner that @@ -97,17 +98,16 @@ def disconnect_rel( elif kind == "element": ifc.run( "geometry.disconnect_element", - relating_element=rel.RelatingElement, - related_element=rel.RelatedElement, + relating_element=subject.RelatingElement, + related_element=subject.RelatedElement, ) elif kind == "mep-pair-fitting": - fitting = rel # slot semantics: the fitting whose removal disconnects the pair. - if skip_elem_recreate and fitting is elem: + if skip_elem_recreate and subject is elem: return - if skip_partner_recreate and fitting is partner: + if skip_partner_recreate and subject is partner: return - fitting_obj = ifc.get_object(fitting) + fitting_obj = ifc.get_object(subject) if fitting_obj is not None: geometry.delete_ifc_object(fitting_obj) else: - raise ValueError(f"Unknown rel kind: {kind!r}") + raise ValueError(f"Unknown kind: {kind!r}") diff --git a/src/bonsai/bonsai/tool/connection.py b/src/bonsai/bonsai/tool/connection.py index 709f277cc5..e433ec605e 100644 --- a/src/bonsai/bonsai/tool/connection.py +++ b/src/bonsai/bonsai/tool/connection.py @@ -18,25 +18,26 @@ # # This file was generated with the assistance of an AI coding tool. -"""Generic discovery of the relation linking two IFC elements. +"""Generic discovery of the connection linking two IFC elements. Used by ``bim.disconnect_elements`` so the operator surface is one operator per disconnect intent (active vs. partner, identified by GlobalId) rather -than one per rel class. The kind label returned alongside the rel lets the -operator dispatch the right post-disconnect cleanup: +than one per rel class. Each lookup returns ``(subject, kind)`` tuples where +``subject`` is the entity whose teardown effects the disconnect: -- ``"path"`` for ``IfcRelConnectsPathElements`` (wall-wall, wall-roof, etc.) -- ``"element-top"`` for ``IfcRelConnectsElements`` with ``Description=="TOP"`` - (the rel kind ``extend_walls_to_underside`` creates) -- ``"element"`` for any other ``IfcRelConnectsElements`` -- ``"mep-pair-fitting"`` for an MEP pair whose disconnect is effected by - removing a bridging ``IfcFlowFitting``. The ``rel`` slot for this kind - carries the fitting entity itself (not a relationship entity) — the - subject whose removal disconnects the pair. ``OBSTRUCTION`` fittings are - excluded; those are removed via ``bim.mep_add_obstruction(mode=REMOVE)``. +- ``"path"`` — ``IfcRelConnectsPathElements`` (wall-wall, wall-roof, etc.). + ``subject`` is the rel; removing it disconnects. +- ``"element-top"`` — ``IfcRelConnectsElements`` with ``Description=="TOP"`` + (created by ``extend_walls_to_underside``). ``subject`` is the rel. +- ``"element"`` — any other ``IfcRelConnectsElements``. ``subject`` is the rel. +- ``"mep-pair-fitting"`` — two MEP elements joined via ``IfcRelConnectsPorts`` + through a single bridging ``IfcFlowFitting``. ``subject`` is the fitting + itself; removing it disconnects. ``OBSTRUCTION`` fittings are excluded + here; those go through ``bim.mep_add_obstruction(mode=REMOVE)``. -Add new rel kinds by extending :py:meth:`Connection.find_rel`. The disconnect -operator's cleanup switch maps each kind to the right post-mutation calls.""" +Add new kinds by extending :py:meth:`Connection.find_rels`. The dispatch in +``bonsai.core.connection.disconnect_rel`` maps each kind to the right +post-mutation cleanup; the AST forward-compat guard enforces coverage.""" from __future__ import annotations @@ -55,13 +56,13 @@ class Connection: elem_a: ifcopenshell.entity_instance, elem_b: ifcopenshell.entity_instance, ) -> list[tuple[ifcopenshell.entity_instance, str]]: - """Return every supported rel linking ``elem_a`` to ``elem_b`` as a - list of ``(rel, kind)`` tuples. Walks both ``ConnectedTo`` and - ``ConnectedFrom`` because either side of the rel can be the relating - element, and the same pair may carry rels authored with opposite - orientations (``disconnect_path``'s ``(relating, related)`` mode only - inspects ``relating.ConnectedTo``, so a single call would miss the - opposite-orientation rel).""" + """Return every supported connection linking ``elem_a`` to ``elem_b`` + as a list of ``(subject, kind)`` tuples — ``subject`` is the entity + whose teardown effects the disconnect (the rel itself for + relationship-kinds, the bridging fitting for ``"mep-pair-fitting"``). + Walks both ``ConnectedTo`` and ``ConnectedFrom`` because either side + of a rel can be the relating element, and the same pair may carry + rels authored with opposite orientations.""" rels: list[tuple[ifcopenshell.entity_instance, str]] = [] seen: set[int] = set() @@ -98,8 +99,8 @@ class Connection: elem_a: ifcopenshell.entity_instance, elem_b: ifcopenshell.entity_instance, ) -> tuple[ifcopenshell.entity_instance | None, str | None]: - """Return the first ``(rel, kind)`` or ``(None, None)``. Cheaper than - ``find_rels`` when callers only need to know whether a connection + """Return the first ``(subject, kind)`` or ``(None, None)``. Cheaper + than ``find_rels`` when callers only need to know whether a connection exists or what kind it is.""" rels = cls.find_rels(elem_a, elem_b) return rels[0] if rels else (None, None) @@ -109,9 +110,10 @@ class Connection: cls, elem: ifcopenshell.entity_instance, ) -> list[tuple[ifcopenshell.entity_instance, str, ifcopenshell.entity_instance]]: - """Return every supported rel touching ``elem`` as ``(rel, kind, partner)`` - triples. ``partner`` is the *other* element on the rel — the side cascade - cleanup must operate on when ``elem`` is being deleted. + """Return every supported connection touching ``elem`` as + ``(subject, kind, partner)`` triples. ``partner`` is the *other* + element on the connection — the side cascade cleanup must operate on + when ``elem`` is being deleted. Mirrors :py:meth:`find_rels`'s relationship-kind taxonomy. Notably does NOT emit ``"mep-pair-fitting"`` triples: ``IfcRelConnectsPorts`` diff --git a/src/bonsai/bonsai/tool/geometry.py b/src/bonsai/bonsai/tool/geometry.py index 911c9990fb..4faae540e7 100644 --- a/src/bonsai/bonsai/tool/geometry.py +++ b/src/bonsai/bonsai/tool/geometry.py @@ -396,13 +396,13 @@ class Geometry(bonsai.core.tool.Geometry): # same OverrideDelete batch. if element.is_a("IfcRoot"): skip_ids = batch_being_deleted_ids or set() - for rel, kind, partner in tool.Connection.find_rels_for_element(element): + for subject, kind, partner in tool.Connection.find_rels_for_element(element): bonsai.core.connection.disconnect_rel( tool.Ifc, tool.Geometry, tool.Model, tool.Connection, - rel=rel, + subject=subject, kind=kind, elem=element, partner=partner, diff --git a/src/bonsai/test/bim/module/model/test_disconnect_elements.py b/src/bonsai/test/bim/module/model/test_disconnect_elements.py index e39bf8af03..09c0c4bf64 100644 --- a/src/bonsai/test/bim/module/model/test_disconnect_elements.py +++ b/src/bonsai/test/bim/module/model/test_disconnect_elements.py @@ -380,9 +380,9 @@ def test_disconnect_dispatches_one_call_per_rel(): assert dispatch.call_count == 2 # Both rels dispatch with elem=elem_a, partner=elem_b regardless of orientation # — orient_element_top inside disconnect_rel recovers the wall/slab roles. - for call, expected_rel, expected_kind in zip(dispatch.call_args_list, [rel1, rel2], ["path", "element-top"]): + for call, expected_subject, expected_kind in zip(dispatch.call_args_list, [rel1, rel2], ["path", "element-top"]): kw = call.kwargs - assert kw["rel"] is expected_rel + assert kw["subject"] is expected_subject assert kw["kind"] == expected_kind assert kw["elem"] is elem_a assert kw["partner"] is elem_b @@ -485,7 +485,7 @@ def test_disconnect_gizmo_direction_symmetry(): # disconnect_rel sees (rel, "element-top") in both runs; elem/partner swap # by argument order but orient_element_top inside disconnect_rel resolves # the wall/slab roles symmetrically. - assert wall_first["rel"] is rel and slab_first["rel"] is rel + assert wall_first["subject"] is rel and slab_first["subject"] is rel assert wall_first["kind"] == slab_first["kind"] == "element-top" assert {wall_first["elem"], wall_first["partner"]} == {wall, slab} assert {slab_first["elem"], slab_first["partner"]} == {wall, slab} diff --git a/src/bonsai/test/bim/module/model/test_mep_disconnect_integration.py b/src/bonsai/test/bim/module/model/test_mep_disconnect_integration.py index 506f26279b..b2af2fa215 100644 --- a/src/bonsai/test/bim/module/model/test_mep_disconnect_integration.py +++ b/src/bonsai/test/bim/module/model/test_mep_disconnect_integration.py @@ -24,7 +24,7 @@ Builds a real IFC scene (two pipe segments joined via ports to a bridging fitting) and exercises the full chain: tool.Connection.find_rels(seg_a, seg_b) → returns (fitting, "mep-pair-fitting") - → bonsai.core.connection.disconnect_rel(rel=fitting, kind=...) + → bonsai.core.connection.disconnect_rel(subject=fitting, kind=...) → tool.Geometry.delete_ifc_object(fitting_obj) → cascade-on-delete removes the IfcRelConnectsPorts via remove_port @@ -100,7 +100,7 @@ class TestMEPPairDisconnectEndToEnd(NewFile): tool.Geometry, tool.Model, tool.Connection, - rel=bend, + subject=bend, kind="mep-pair-fitting", elem=seg_a, partner=seg_b, @@ -127,7 +127,7 @@ class TestMEPPairDisconnectEndToEnd(NewFile): tool.Geometry, tool.Model, tool.Connection, - rel=bend, + subject=bend, kind="mep-pair-fitting", elem=bend, partner=seg_a, diff --git a/src/bonsai/test/core/test_connection.py b/src/bonsai/test/core/test_connection.py index 293bdab05a..4e87a65f74 100644 --- a/src/bonsai/test/core/test_connection.py +++ b/src/bonsai/test/core/test_connection.py @@ -64,7 +64,7 @@ class TestDisconnectRelPath: geometry, model, connection, - rel="rel", + subject="rel", kind="path", elem="elem_a", partner="elem_b", @@ -88,7 +88,7 @@ class TestDisconnectRelPath: geometry, model, connection, - rel="rel", + subject="rel", kind="path", elem="elem", partner="partner", @@ -109,7 +109,7 @@ class TestDisconnectRelPath: geometry, model, connection, - rel="rel", + subject="rel", kind="path", elem="elem", partner="partner", @@ -130,7 +130,7 @@ class TestDisconnectRelPath: geometry, model, connection, - rel="rel", + subject="rel", kind="path", elem="elem", partner="partner", @@ -159,7 +159,7 @@ class TestDisconnectRelElementTop: geometry, model, connection, - rel=rel, + subject=rel, kind="element-top", elem="elem", partner="partner", @@ -182,7 +182,7 @@ class TestDisconnectRelElementTop: Mock(), Mock(), connection, - rel=rel, + subject=rel, kind="element-top", elem="slab", partner="wall", @@ -205,7 +205,7 @@ class TestDisconnectRelElementTop: Mock(), Mock(), connection, - rel=rel, + subject=rel, kind="element-top", elem="wall", partner="slab", @@ -229,7 +229,7 @@ class TestDisconnectRelElementTop: Mock(), Mock(), connection, - rel=rel, + subject=rel, kind="element-top", elem="slab", partner="wall", @@ -250,7 +250,7 @@ class TestDisconnectRelElement: Mock(), Mock(), Mock(), - rel=rel, + subject=rel, kind="element", elem="elem_a", partner="elem_b", @@ -276,7 +276,7 @@ class TestDisconnectRelMEPPairFitting: geometry, Mock(), Mock(), - rel=fitting, + subject=fitting, kind="mep-pair-fitting", elem="seg_a", partner="seg_b", @@ -296,7 +296,7 @@ class TestDisconnectRelMEPPairFitting: geometry, Mock(), Mock(), - rel=fitting, + subject=fitting, kind="mep-pair-fitting", elem="seg_a", partner="seg_b", @@ -316,7 +316,7 @@ class TestDisconnectRelMEPPairFitting: geometry, Mock(), Mock(), - rel=fitting, + subject=fitting, kind="mep-pair-fitting", elem=fitting, partner="other", @@ -335,7 +335,7 @@ class TestDisconnectRelMEPPairFitting: geometry, Mock(), Mock(), - rel=fitting, + subject=fitting, kind="mep-pair-fitting", elem="seg_a", partner=fitting, @@ -347,13 +347,13 @@ class TestDisconnectRelMEPPairFitting: class TestDisconnectRelUnknownKind: def test_raises_value_error(self): - with pytest.raises(ValueError, match="Unknown rel kind"): + with pytest.raises(ValueError, match="Unknown kind"): subject.disconnect_rel( Mock(), Mock(), Mock(), Mock(), - rel="rel", + subject="rel", kind="bogus", elem="a", partner="b", diff --git a/src/bonsai/test/tool/test_connection_forward_compat.py b/src/bonsai/test/tool/test_connection_forward_compat.py index af830353b3..b73ac71947 100644 --- a/src/bonsai/test/tool/test_connection_forward_compat.py +++ b/src/bonsai/test/tool/test_connection_forward_compat.py @@ -19,12 +19,12 @@ # This file was generated with the assistance of an AI coding tool. """Forward-compat AST contract: ``core.connection.disconnect_rel`` must have a -branch for every rel ``kind`` emitted by ``tool.connection.Connection`` lookups. +branch for every ``kind`` emitted by ``tool.connection.Connection`` lookups. -Adding a new rel kind (e.g. ``"void"``, ``"fill"``, ``"interferes"``) to +Adding a new kind (e.g. ``"void"``, ``"fill"``, ``"interferes"``) to ``find_rels`` / ``find_rels_for_element`` without extending ``disconnect_rel`` would silently regress the disconnect operator and the cascade-on-delete: a new -kind would reach the dispatch, hit the ``raise ValueError("Unknown rel kind")`` +kind would reach the dispatch, hit the ``raise ValueError("Unknown kind")`` fallback, and either crash the operator or leave the cascade half-done. This guard makes the symmetry mandatory at test time."""