Refactor: rename rel -> subject in disconnect_rel dispatch

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.
This commit is contained in:
Gorgious56
2026-06-23 14:27:29 +02:00
parent 6fa984ce2b
commit 006a24ef32
8 changed files with 76 additions and 74 deletions
+2 -2
View File
@@ -400,13 +400,13 @@ class DisconnectElements(_CommitWallDraftsFirstMixin, bpy.types.Operator, tool.I
) )
return return
path_objs: list[bpy.types.Object] = [] path_objs: list[bpy.types.Object] = []
for rel, kind in rels: for subject, kind in rels:
bonsai.core.connection.disconnect_rel( bonsai.core.connection.disconnect_rel(
tool.Ifc, tool.Ifc,
tool.Geometry, tool.Geometry,
tool.Model, tool.Model,
tool.Connection, tool.Connection,
rel=rel, subject=subject,
kind=kind, kind=kind,
elem=elem_a, elem=elem_a,
partner=elem_b, partner=elem_b,
+20 -20
View File
@@ -22,17 +22,18 @@
Used by both ``bim.disconnect_elements`` (explicit user disconnect) and the Used by both ``bim.disconnect_elements`` (explicit user disconnect) and the
connection cascade in ``tool.Geometry.delete_ifc_object`` (implicit 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:`bonsai.tool.connection.Connection.find_rels` /
:py:meth:`find_rels_for_element` maps to a single arm here, so adding a new :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. automatically and the AST forward-compat guard enforces coverage.
For the ``"mep-pair-fitting"`` kind the ``rel`` slot carries the The ``subject`` parameter is the entity whose teardown effects the
``IfcFlowFitting`` itself rather than a relationship entity the subject disconnect: for ``"path"`` / ``"element"`` / ``"element-top"`` kinds it
whose removal disconnects the pair. The dispatch treats the slot as the carries an ``IfcRel*`` relationship entity (the rel that gets removed);
deletion target and routes through ``geometry.delete_ifc_object`` so the for ``"mep-pair-fitting"`` it carries an ``IfcFlowFitting`` (the fitting
cascade-on-delete contract still owns port-rel cleanup. that gets deleted). The slot is uniform on intent the dispatch decides
the teardown mechanism by kind.
""" """
from __future__ import annotations from __future__ import annotations
@@ -53,14 +54,14 @@ def disconnect_rel(
geometry: type[tool.Geometry], geometry: type[tool.Geometry],
model: type[tool.Model], model: type[tool.Model],
connection: type[tool.Connection], connection: type[tool.Connection],
rel: ifcopenshell.entity_instance, subject: ifcopenshell.entity_instance,
kind: str, kind: str,
elem: ifcopenshell.entity_instance, elem: ifcopenshell.entity_instance,
partner: ifcopenshell.entity_instance, partner: ifcopenshell.entity_instance,
skip_elem_recreate: bool = False, skip_elem_recreate: bool = False,
skip_partner_recreate: bool = False, skip_partner_recreate: bool = False,
) -> None: ) -> 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`` ``elem`` and ``partner`` are the two endpoints. The ``skip_*_recreate``
flags suppress per-side regenerate / recreate work used by the flags suppress per-side regenerate / recreate work used by the
@@ -70,7 +71,7 @@ def disconnect_rel(
runs on both sides. runs on both sides.
""" """
if kind == "path": if kind == "path":
bonsai.core.geometry.remove_connection(geometry, connection=rel) bonsai.core.geometry.remove_connection(geometry, connection=subject)
if not skip_elem_recreate: if not skip_elem_recreate:
elem_obj = ifc.get_object(elem) elem_obj = ifc.get_object(elem)
if elem_obj is not None: if elem_obj is not None:
@@ -80,11 +81,11 @@ def disconnect_rel(
if partner_obj is not None: if partner_obj is not None:
model.recreate_wall(partner, partner_obj) model.recreate_wall(partner, partner_obj)
elif kind == "element-top": elif kind == "element-top":
wall, _slab = connection.orient_element_top(rel, elem, partner) wall, _slab = connection.orient_element_top(subject, elem, partner)
ifc.run( ifc.run(
"geometry.disconnect_element", "geometry.disconnect_element",
relating_element=rel.RelatingElement, relating_element=subject.RelatingElement,
related_element=rel.RelatedElement, related_element=subject.RelatedElement,
) )
# Skip the wall-side regenerate when the wall is itself being deleted — # 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 # 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": elif kind == "element":
ifc.run( ifc.run(
"geometry.disconnect_element", "geometry.disconnect_element",
relating_element=rel.RelatingElement, relating_element=subject.RelatingElement,
related_element=rel.RelatedElement, related_element=subject.RelatedElement,
) )
elif kind == "mep-pair-fitting": elif kind == "mep-pair-fitting":
fitting = rel # slot semantics: the fitting whose removal disconnects the pair. if skip_elem_recreate and subject is elem:
if skip_elem_recreate and fitting is elem:
return return
if skip_partner_recreate and fitting is partner: if skip_partner_recreate and subject is partner:
return return
fitting_obj = ifc.get_object(fitting) fitting_obj = ifc.get_object(subject)
if fitting_obj is not None: if fitting_obj is not None:
geometry.delete_ifc_object(fitting_obj) geometry.delete_ifc_object(fitting_obj)
else: else:
raise ValueError(f"Unknown rel kind: {kind!r}") raise ValueError(f"Unknown kind: {kind!r}")
+28 -26
View File
@@ -18,25 +18,26 @@
# #
# This file was generated with the assistance of an AI coding tool. # 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 Used by ``bim.disconnect_elements`` so the operator surface is one operator
per disconnect intent (active vs. partner, identified by GlobalId) rather per disconnect intent (active vs. partner, identified by GlobalId) rather
than one per rel class. The kind label returned alongside the rel lets the than one per rel class. Each lookup returns ``(subject, kind)`` tuples where
operator dispatch the right post-disconnect cleanup: ``subject`` is the entity whose teardown effects the disconnect:
- ``"path"`` for ``IfcRelConnectsPathElements`` (wall-wall, wall-roof, etc.) - ``"path"`` ``IfcRelConnectsPathElements`` (wall-wall, wall-roof, etc.).
- ``"element-top"`` for ``IfcRelConnectsElements`` with ``Description=="TOP"`` ``subject`` is the rel; removing it disconnects.
(the rel kind ``extend_walls_to_underside`` creates) - ``"element-top"`` ``IfcRelConnectsElements`` with ``Description=="TOP"``
- ``"element"`` for any other ``IfcRelConnectsElements`` (created by ``extend_walls_to_underside``). ``subject`` is the rel.
- ``"mep-pair-fitting"`` for an MEP pair whose disconnect is effected by - ``"element"`` any other ``IfcRelConnectsElements``. ``subject`` is the rel.
removing a bridging ``IfcFlowFitting``. The ``rel`` slot for this kind - ``"mep-pair-fitting"`` two MEP elements joined via ``IfcRelConnectsPorts``
carries the fitting entity itself (not a relationship entity) the through a single bridging ``IfcFlowFitting``. ``subject`` is the fitting
subject whose removal disconnects the pair. ``OBSTRUCTION`` fittings are itself; removing it disconnects. ``OBSTRUCTION`` fittings are excluded
excluded; those are removed via ``bim.mep_add_obstruction(mode=REMOVE)``. here; those go through ``bim.mep_add_obstruction(mode=REMOVE)``.
Add new rel kinds by extending :py:meth:`Connection.find_rel`. The disconnect Add new kinds by extending :py:meth:`Connection.find_rels`. The dispatch in
operator's cleanup switch maps each kind to the right post-mutation calls.""" ``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 from __future__ import annotations
@@ -55,13 +56,13 @@ class Connection:
elem_a: ifcopenshell.entity_instance, elem_a: ifcopenshell.entity_instance,
elem_b: ifcopenshell.entity_instance, elem_b: ifcopenshell.entity_instance,
) -> list[tuple[ifcopenshell.entity_instance, str]]: ) -> list[tuple[ifcopenshell.entity_instance, str]]:
"""Return every supported rel linking ``elem_a`` to ``elem_b`` as a """Return every supported connection linking ``elem_a`` to ``elem_b``
list of ``(rel, kind)`` tuples. Walks both ``ConnectedTo`` and as a list of ``(subject, kind)`` tuples ``subject`` is the entity
``ConnectedFrom`` because either side of the rel can be the relating whose teardown effects the disconnect (the rel itself for
element, and the same pair may carry rels authored with opposite relationship-kinds, the bridging fitting for ``"mep-pair-fitting"``).
orientations (``disconnect_path``'s ``(relating, related)`` mode only Walks both ``ConnectedTo`` and ``ConnectedFrom`` because either side
inspects ``relating.ConnectedTo``, so a single call would miss the of a rel can be the relating element, and the same pair may carry
opposite-orientation rel).""" rels authored with opposite orientations."""
rels: list[tuple[ifcopenshell.entity_instance, str]] = [] rels: list[tuple[ifcopenshell.entity_instance, str]] = []
seen: set[int] = set() seen: set[int] = set()
@@ -98,8 +99,8 @@ class Connection:
elem_a: ifcopenshell.entity_instance, elem_a: ifcopenshell.entity_instance,
elem_b: ifcopenshell.entity_instance, elem_b: ifcopenshell.entity_instance,
) -> tuple[ifcopenshell.entity_instance | None, str | None]: ) -> tuple[ifcopenshell.entity_instance | None, str | None]:
"""Return the first ``(rel, kind)`` or ``(None, None)``. Cheaper than """Return the first ``(subject, kind)`` or ``(None, None)``. Cheaper
``find_rels`` when callers only need to know whether a connection than ``find_rels`` when callers only need to know whether a connection
exists or what kind it is.""" exists or what kind it is."""
rels = cls.find_rels(elem_a, elem_b) rels = cls.find_rels(elem_a, elem_b)
return rels[0] if rels else (None, None) return rels[0] if rels else (None, None)
@@ -109,9 +110,10 @@ class Connection:
cls, cls,
elem: ifcopenshell.entity_instance, elem: ifcopenshell.entity_instance,
) -> list[tuple[ifcopenshell.entity_instance, str, ifcopenshell.entity_instance]]: ) -> list[tuple[ifcopenshell.entity_instance, str, ifcopenshell.entity_instance]]:
"""Return every supported rel touching ``elem`` as ``(rel, kind, partner)`` """Return every supported connection touching ``elem`` as
triples. ``partner`` is the *other* element on the rel the side cascade ``(subject, kind, partner)`` triples. ``partner`` is the *other*
cleanup must operate on when ``elem`` is being deleted. 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 Mirrors :py:meth:`find_rels`'s relationship-kind taxonomy. Notably
does NOT emit ``"mep-pair-fitting"`` triples: ``IfcRelConnectsPorts`` does NOT emit ``"mep-pair-fitting"`` triples: ``IfcRelConnectsPorts``
+2 -2
View File
@@ -396,13 +396,13 @@ class Geometry(bonsai.core.tool.Geometry):
# same OverrideDelete batch. # same OverrideDelete batch.
if element.is_a("IfcRoot"): if element.is_a("IfcRoot"):
skip_ids = batch_being_deleted_ids or set() 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( bonsai.core.connection.disconnect_rel(
tool.Ifc, tool.Ifc,
tool.Geometry, tool.Geometry,
tool.Model, tool.Model,
tool.Connection, tool.Connection,
rel=rel, subject=subject,
kind=kind, kind=kind,
elem=element, elem=element,
partner=partner, partner=partner,
@@ -380,9 +380,9 @@ def test_disconnect_dispatches_one_call_per_rel():
assert dispatch.call_count == 2 assert dispatch.call_count == 2
# Both rels dispatch with elem=elem_a, partner=elem_b regardless of orientation # Both rels dispatch with elem=elem_a, partner=elem_b regardless of orientation
# — orient_element_top inside disconnect_rel recovers the wall/slab roles. # — 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 kw = call.kwargs
assert kw["rel"] is expected_rel assert kw["subject"] is expected_subject
assert kw["kind"] == expected_kind assert kw["kind"] == expected_kind
assert kw["elem"] is elem_a assert kw["elem"] is elem_a
assert kw["partner"] is elem_b 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 # disconnect_rel sees (rel, "element-top") in both runs; elem/partner swap
# by argument order but orient_element_top inside disconnect_rel resolves # by argument order but orient_element_top inside disconnect_rel resolves
# the wall/slab roles symmetrically. # 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["kind"] == slab_first["kind"] == "element-top"
assert {wall_first["elem"], wall_first["partner"]} == {wall, slab} assert {wall_first["elem"], wall_first["partner"]} == {wall, slab}
assert {slab_first["elem"], slab_first["partner"]} == {wall, slab} assert {slab_first["elem"], slab_first["partner"]} == {wall, slab}
@@ -24,7 +24,7 @@ Builds a real IFC scene (two pipe segments joined via ports to a bridging
fitting) and exercises the full chain: fitting) and exercises the full chain:
tool.Connection.find_rels(seg_a, seg_b) tool.Connection.find_rels(seg_a, seg_b)
returns (fitting, "mep-pair-fitting") 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) tool.Geometry.delete_ifc_object(fitting_obj)
cascade-on-delete removes the IfcRelConnectsPorts via remove_port cascade-on-delete removes the IfcRelConnectsPorts via remove_port
@@ -100,7 +100,7 @@ class TestMEPPairDisconnectEndToEnd(NewFile):
tool.Geometry, tool.Geometry,
tool.Model, tool.Model,
tool.Connection, tool.Connection,
rel=bend, subject=bend,
kind="mep-pair-fitting", kind="mep-pair-fitting",
elem=seg_a, elem=seg_a,
partner=seg_b, partner=seg_b,
@@ -127,7 +127,7 @@ class TestMEPPairDisconnectEndToEnd(NewFile):
tool.Geometry, tool.Geometry,
tool.Model, tool.Model,
tool.Connection, tool.Connection,
rel=bend, subject=bend,
kind="mep-pair-fitting", kind="mep-pair-fitting",
elem=bend, elem=bend,
partner=seg_a, partner=seg_a,
+15 -15
View File
@@ -64,7 +64,7 @@ class TestDisconnectRelPath:
geometry, geometry,
model, model,
connection, connection,
rel="rel", subject="rel",
kind="path", kind="path",
elem="elem_a", elem="elem_a",
partner="elem_b", partner="elem_b",
@@ -88,7 +88,7 @@ class TestDisconnectRelPath:
geometry, geometry,
model, model,
connection, connection,
rel="rel", subject="rel",
kind="path", kind="path",
elem="elem", elem="elem",
partner="partner", partner="partner",
@@ -109,7 +109,7 @@ class TestDisconnectRelPath:
geometry, geometry,
model, model,
connection, connection,
rel="rel", subject="rel",
kind="path", kind="path",
elem="elem", elem="elem",
partner="partner", partner="partner",
@@ -130,7 +130,7 @@ class TestDisconnectRelPath:
geometry, geometry,
model, model,
connection, connection,
rel="rel", subject="rel",
kind="path", kind="path",
elem="elem", elem="elem",
partner="partner", partner="partner",
@@ -159,7 +159,7 @@ class TestDisconnectRelElementTop:
geometry, geometry,
model, model,
connection, connection,
rel=rel, subject=rel,
kind="element-top", kind="element-top",
elem="elem", elem="elem",
partner="partner", partner="partner",
@@ -182,7 +182,7 @@ class TestDisconnectRelElementTop:
Mock(), Mock(),
Mock(), Mock(),
connection, connection,
rel=rel, subject=rel,
kind="element-top", kind="element-top",
elem="slab", elem="slab",
partner="wall", partner="wall",
@@ -205,7 +205,7 @@ class TestDisconnectRelElementTop:
Mock(), Mock(),
Mock(), Mock(),
connection, connection,
rel=rel, subject=rel,
kind="element-top", kind="element-top",
elem="wall", elem="wall",
partner="slab", partner="slab",
@@ -229,7 +229,7 @@ class TestDisconnectRelElementTop:
Mock(), Mock(),
Mock(), Mock(),
connection, connection,
rel=rel, subject=rel,
kind="element-top", kind="element-top",
elem="slab", elem="slab",
partner="wall", partner="wall",
@@ -250,7 +250,7 @@ class TestDisconnectRelElement:
Mock(), Mock(),
Mock(), Mock(),
Mock(), Mock(),
rel=rel, subject=rel,
kind="element", kind="element",
elem="elem_a", elem="elem_a",
partner="elem_b", partner="elem_b",
@@ -276,7 +276,7 @@ class TestDisconnectRelMEPPairFitting:
geometry, geometry,
Mock(), Mock(),
Mock(), Mock(),
rel=fitting, subject=fitting,
kind="mep-pair-fitting", kind="mep-pair-fitting",
elem="seg_a", elem="seg_a",
partner="seg_b", partner="seg_b",
@@ -296,7 +296,7 @@ class TestDisconnectRelMEPPairFitting:
geometry, geometry,
Mock(), Mock(),
Mock(), Mock(),
rel=fitting, subject=fitting,
kind="mep-pair-fitting", kind="mep-pair-fitting",
elem="seg_a", elem="seg_a",
partner="seg_b", partner="seg_b",
@@ -316,7 +316,7 @@ class TestDisconnectRelMEPPairFitting:
geometry, geometry,
Mock(), Mock(),
Mock(), Mock(),
rel=fitting, subject=fitting,
kind="mep-pair-fitting", kind="mep-pair-fitting",
elem=fitting, elem=fitting,
partner="other", partner="other",
@@ -335,7 +335,7 @@ class TestDisconnectRelMEPPairFitting:
geometry, geometry,
Mock(), Mock(),
Mock(), Mock(),
rel=fitting, subject=fitting,
kind="mep-pair-fitting", kind="mep-pair-fitting",
elem="seg_a", elem="seg_a",
partner=fitting, partner=fitting,
@@ -347,13 +347,13 @@ class TestDisconnectRelMEPPairFitting:
class TestDisconnectRelUnknownKind: class TestDisconnectRelUnknownKind:
def test_raises_value_error(self): def test_raises_value_error(self):
with pytest.raises(ValueError, match="Unknown rel kind"): with pytest.raises(ValueError, match="Unknown kind"):
subject.disconnect_rel( subject.disconnect_rel(
Mock(), Mock(),
Mock(), Mock(),
Mock(), Mock(),
Mock(), Mock(),
rel="rel", subject="rel",
kind="bogus", kind="bogus",
elem="a", elem="a",
partner="b", partner="b",
@@ -19,12 +19,12 @@
# This file was generated with the assistance of an AI coding tool. # This file was generated with the assistance of an AI coding tool.
"""Forward-compat AST contract: ``core.connection.disconnect_rel`` must have a """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`` ``find_rels`` / ``find_rels_for_element`` without extending ``disconnect_rel``
would silently regress the disconnect operator and the cascade-on-delete: a new 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 fallback, and either crash the operator or leave the cascade half-done. This
guard makes the symmetry mandatory at test time.""" guard makes the symmetry mandatory at test time."""