mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-25 17:57:02 +00:00
ifcpatch: fix MergeDuplicateTypes destroying distinct/cross-class types
MergeDuplicateTypes grouped IfcTypeObjects by a single attribute value
only, which caused two failures:
- With should_merge_null=True (the old default), every type with an empty
Tag was treated as a duplicate and collapsed into one. On Bonsai-authored
or mixed models this silently destroyed distinct, differently-named types
(e.g. array-child proxy types), leaving BBIM_Array parents pointing at
removed child GUIDs ("Arrays With Missing Children" on reload).
- Ignoring the IFC class let unrelated types merge, raising e.g.
"IfcTypeProduct cannot type IfcBeam" from assign_type.
Include the IFC class in the merge key so only same-class types merge, and
default should_merge_null to False so untagged types are kept separate
unless explicitly opted in via ["Tag", True]. Genuine Revit duplicates
always carry a populated Tag, so the target workflow is unaffected.
Adds regression tests: distinct null-attribute types preserved by default,
and cross-class types never merged.
See https://github.com/IfcOpenShell/IfcOpenShell/issues/8609
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -25,7 +25,7 @@ import ifcopenshell.util.element
|
|||||||
|
|
||||||
|
|
||||||
class Patcher:
|
class Patcher:
|
||||||
def __init__(self, file: ifcopenshell.file, logger: Logger, attribute: str = "Tag", should_merge_null: bool = True):
|
def __init__(self, file: ifcopenshell.file, logger: Logger, attribute: str = "Tag", should_merge_null: bool = False):
|
||||||
"""Merge duplicate element types via the Tag or another attribute
|
"""Merge duplicate element types via the Tag or another attribute
|
||||||
|
|
||||||
Revit is notorious for creating many duplicate element types. Element
|
Revit is notorious for creating many duplicate element types. Element
|
||||||
@@ -50,8 +50,18 @@ class Patcher:
|
|||||||
:param attribute: The name of the attribute to merge element types based
|
:param attribute: The name of the attribute to merge element types based
|
||||||
on. Typically this will be "Tag" as it stores the unique ID from the
|
on. Typically this will be "Tag" as it stores the unique ID from the
|
||||||
proprietary BIM software.
|
proprietary BIM software.
|
||||||
:param should_merge_null: If True, all elements with an empty attribute
|
Only types of the same IFC class are ever merged, so e.g. an
|
||||||
will be merged. If False, they will be kept separate.
|
annotation type and a beam type that happen to share an attribute value
|
||||||
|
are never combined.
|
||||||
|
|
||||||
|
:param should_merge_null: If True, all types with an empty attribute
|
||||||
|
will be merged together. This defaults to False because an empty
|
||||||
|
attribute is an absence of evidence, not proof of duplication:
|
||||||
|
merging every untagged type into one silently destroys distinct
|
||||||
|
types (a common failure on Bonsai-authored or mixed models, where
|
||||||
|
genuine, differently-named types may all lack a Tag). Genuine Revit
|
||||||
|
duplicates always carry a populated Tag, so the default only affects
|
||||||
|
types the recipe has no reason to believe are duplicates.
|
||||||
|
|
||||||
Example:
|
Example:
|
||||||
|
|
||||||
@@ -62,6 +72,9 @@ class Patcher:
|
|||||||
|
|
||||||
# Explicitly say we want to merge based on the Name attribute
|
# Explicitly say we want to merge based on the Name attribute
|
||||||
ifcpatch.execute({"file": model, "recipe": "MergeDuplicateTypes", "arguments": ["Name"]})
|
ifcpatch.execute({"file": model, "recipe": "MergeDuplicateTypes", "arguments": ["Name"]})
|
||||||
|
|
||||||
|
# Also merge all types that have an empty Tag into a single type
|
||||||
|
ifcpatch.execute({"file": model, "recipe": "MergeDuplicateTypes", "arguments": ["Tag", True]})
|
||||||
"""
|
"""
|
||||||
self.file = file
|
self.file = file
|
||||||
self.logger = logger
|
self.logger = logger
|
||||||
@@ -71,9 +84,15 @@ class Patcher:
|
|||||||
def patch(self):
|
def patch(self):
|
||||||
keys: dict[Any, ifcopenshell.entity_instance] = {}
|
keys: dict[Any, ifcopenshell.entity_instance] = {}
|
||||||
for element_type in self.file.by_type("IfcTypeObject"):
|
for element_type in self.file.by_type("IfcTypeObject"):
|
||||||
key = getattr(element_type, self.attribute)
|
attribute_value = getattr(element_type, self.attribute)
|
||||||
if not key and not self.should_merge_null:
|
if not attribute_value and not self.should_merge_null:
|
||||||
continue
|
continue
|
||||||
|
# Include the IFC class in the key so that only types of the same
|
||||||
|
# class are ever merged. Two types of different classes are not
|
||||||
|
# duplicates, and merging them would reassign occurrences to an
|
||||||
|
# incompatible type (e.g. an IfcTypeProduct used for annotations
|
||||||
|
# cannot type an IfcBeam), raising a TypeError in assign_type.
|
||||||
|
key = (element_type.is_a(), attribute_value)
|
||||||
original_type = keys.get(key, None)
|
original_type = keys.get(key, None)
|
||||||
if original_type:
|
if original_type:
|
||||||
elements = ifcopenshell.util.element.get_types(element_type)
|
elements = ifcopenshell.util.element.get_types(element_type)
|
||||||
|
|||||||
@@ -72,6 +72,35 @@ class TestMergeDuplicateTypes(test.bootstrap.IFC4):
|
|||||||
assert ifcopenshell.util.element.get_material(wall1, should_inherit=False) == None
|
assert ifcopenshell.util.element.get_material(wall1, should_inherit=False) == None
|
||||||
assert ifcopenshell.util.element.get_material(wall2, should_inherit=False) == None
|
assert ifcopenshell.util.element.get_material(wall2, should_inherit=False) == None
|
||||||
|
|
||||||
|
def test_not_merging_different_classes(self):
|
||||||
|
# Types of different classes are not duplicates even if they share the
|
||||||
|
# merge attribute (here an empty Tag). Merging them would try to
|
||||||
|
# reassign occurrences to an incompatible type. See the IfcTypeProduct
|
||||||
|
# (annotation) vs IfcBeam case.
|
||||||
|
annotation_type = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcTypeProduct")
|
||||||
|
annotation = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcAnnotation")
|
||||||
|
ifcopenshell.api.type.assign_type(
|
||||||
|
self.file, related_objects=[annotation], relating_type=annotation_type, should_map_representations=False
|
||||||
|
)
|
||||||
|
beam_type = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcBeamType")
|
||||||
|
beam = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcBeam")
|
||||||
|
ifcopenshell.api.type.assign_type(
|
||||||
|
self.file, related_objects=[beam], relating_type=beam_type, should_map_representations=False
|
||||||
|
)
|
||||||
|
output = ifcpatch.execute({"file": self.file, "recipe": "MergeDuplicateTypes", "arguments": ["Tag", True]})
|
||||||
|
assert len(output.by_type("IfcTypeProduct")) == 2
|
||||||
|
assert output.by_type("IfcBeamType")
|
||||||
|
|
||||||
|
def test_empty_attributes_kept_separate_by_default(self):
|
||||||
|
# An empty attribute is an absence of evidence, not proof of
|
||||||
|
# duplication. By default (should_merge_null=False) distinct types that
|
||||||
|
# merely lack a Tag must be preserved, otherwise e.g. differently-named
|
||||||
|
# Bonsai-authored types get silently collapsed into one.
|
||||||
|
for name in ("Wire Ladder", "CMU Core", "Mortar Joint"):
|
||||||
|
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcBuildingElementProxyType", name=name)
|
||||||
|
output = ifcpatch.execute({"file": self.file, "recipe": "MergeDuplicateTypes", "arguments": []})
|
||||||
|
assert len(output.by_type("IfcBuildingElementProxyType")) == 3
|
||||||
|
|
||||||
def test_not_merging_empty_attributes(self):
|
def test_not_merging_empty_attributes(self):
|
||||||
wall_type1 = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWallType", name="")
|
wall_type1 = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWallType", name="")
|
||||||
wall_type2 = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWallType", name="")
|
wall_type2 = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWallType", name="")
|
||||||
|
|||||||
Reference in New Issue
Block a user