See #3334. Fix bug where get_pset didn't handle property inheritance correctly.

src/ifcopenshell-python/ifcopenshell/util/element.py

@@ -25,6 +25,9 @@ def get_pset(element, name, prop=None, should_inherit=True):
     This is more efficient than ifcopenshell.util.element.get_psets if you know
     exactly which property set and property you are after.
 
+    If should_inherit is true, the pset "id" only refers to the ID of the
+    occurrence, not the type's pset.
+
     :param element: The IFC Element entity
     :type element: ifcopenshell.entity_instance.entity_instance
     :param name: The name of the pset
@@ -45,6 +48,8 @@ def get_pset(element, name, prop=None, should_inherit=True):
         psets_and_qtos = ifcopenshell.util.element.get_pset(element, "Pset_WallCommon")
     """
     pset = None
+    type_pset = None
+
     if element.is_a("IfcTypeObject"):
         for definition in element.HasPropertySets or []:
             if definition.Name == name:
@@ -56,11 +61,10 @@ def get_pset(element, name, prop=None, should_inherit=True):
                 pset = definition
                 break
     elif hasattr(element, "IsDefinedBy"):
-        element_type = ifcopenshell.util.element.get_type(element)
-        if element_type and should_inherit:
-            result = get_pset(element_type, name, prop, should_inherit=False)
-            if result:
-                return result
+        if should_inherit:
+            element_type = ifcopenshell.util.element.get_type(element)
+            if element_type:
+                type_pset = get_pset(element_type, name, prop, should_inherit=False)
         for relationship in element.IsDefinedBy:
             if relationship.is_a("IfcRelDefinesByProperties"):
                 definition = relationship.RelatingPropertyDefinition
@@ -68,30 +72,28 @@ def get_pset(element, name, prop=None, should_inherit=True):
                     pset = definition
                     break
 
-    if not pset:
-        return
+    if not pset and not type_pset:
+        return {}
 
     if not prop:
+        if type_pset:
+            type_pset.update(get_property_definition(pset))
+            return type_pset
         return get_property_definition(pset)
 
-    if definition.is_a("IfcElementQuantity"):
-        return get_quantity(definition.Quantities, prop)
-    elif definition.is_a("IfcPropertySet"):
-        return get_property(definition.HasProperties, prop)
-    elif definition.is_a("IfcMaterialProperties") or definition.is_a("IfcProfileProperties"):
-        return get_property(definition.Properties, prop)
-    else:
-        # Entity introduced in IFC4
-        # definition.is_a('IfcPreDefinedPropertySet'):
-        for i in range(4, len(definition)):
-            if definition[i] is not None:
-                if definition.attribute_name(i) == prop:
-                    return definition[i]
-
+    value = get_property_definition(pset, prop)
+    if type_pset:
+        type_value = get_property_definition(type_pset, prop)
+        if value is None and type_value is not None:
+            return type_value
+    return value
 
 def get_psets(element, psets_only=False, qtos_only=False, should_inherit=True):
     """Retrieve property sets, their related properties' names & values and ids.
 
+    If should_inherit is true, the pset "id" only refers to the ID of the
+    occurrence, not the type's pset.
+
     :param element: The IFC Element entity
     :type element: ifcopenshell.entity_instance.entity_instance
     :param psets_only: Default as False. Set to true if only property sets are needed.
@@ -140,23 +142,41 @@ def get_psets(element, psets_only=False, qtos_only=False, should_inherit=True):
     return psets
 
 
-def get_property_definition(definition):
-    if definition is not None:
-        props = {}
+def get_property_definition(definition, prop=None):
+    if not definition:
+        return
+
+    if prop:
         if definition.is_a("IfcElementQuantity"):
-            props.update(get_quantities(definition.Quantities))
+            return get_quantity(definition.Quantities, prop)
         elif definition.is_a("IfcPropertySet"):
-            props.update(get_properties(definition.HasProperties))
+            return get_property(definition.HasProperties, prop)
         elif definition.is_a("IfcMaterialProperties") or definition.is_a("IfcProfileProperties"):
-            props.update(get_properties(definition.Properties))
+            return get_property(definition.Properties, prop)
         else:
             # Entity introduced in IFC4
             # definition.is_a('IfcPreDefinedPropertySet'):
-            for prop in range(4, len(definition)):
-                if definition[prop] is not None:
-                    props[definition.attribute_name(prop)] = definition[prop]
-        props["id"] = definition.id()
-        return props
+            for i in range(4, len(definition)):
+                if definition[i] is not None:
+                    if definition.attribute_name(i) == prop:
+                        return definition[i]
+        return
+
+    props = {}
+    if definition.is_a("IfcElementQuantity"):
+        props.update(get_quantities(definition.Quantities))
+    elif definition.is_a("IfcPropertySet"):
+        props.update(get_properties(definition.HasProperties))
+    elif definition.is_a("IfcMaterialProperties") or definition.is_a("IfcProfileProperties"):
+        props.update(get_properties(definition.Properties))
+    else:
+        # Entity introduced in IFC4
+        # definition.is_a('IfcPreDefinedPropertySet'):
+        for prop in range(4, len(definition)):
+            if definition[prop] is not None:
+                props[definition.attribute_name(prop)] = definition[prop]
+    props["id"] = definition.id()
+    return props
 
 
 def get_quantity(quantities, name):

src/ifcopenshell-python/test/util/test_element.py

@@ -22,6 +22,67 @@ import ifcopenshell.api
 import ifcopenshell.util.element as subject
 
 
+class TestGetPsetIFC4(test.bootstrap.IFC4):
+    def test_getting_the_psets_of_a_product_as_a_dictionary(self):
+        element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall")
+        assert subject.get_pset(element, "name") == {}
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=element, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"a": "b"})
+        assert subject.get_pset(element, "name") == {"a": "b", "id": pset.id()}
+
+    def test_getting_the_psets_of_a_product_type_as_a_dictionary(self):
+        type_element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType")
+        assert subject.get_psets(type_element) == {}
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=type_element, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"x": "y"})
+        assert subject.get_pset(type_element, "name") == {"x": "y", "id": pset.id()}
+
+    def test_getting_inherited_psets(self):
+        element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall")
+        type_element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType")
+        ifcopenshell.api.run("type.assign_type", self.file, related_object=element, relating_type=type_element)
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=type_element, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"a": 1, "x": 1})
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=element, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"a": 2, "b": 3})
+        result = subject.get_pset(element, "name")
+        assert result["id"] == pset.id()
+        assert result["a"] == 2
+        assert result["x"] == 1
+        assert result["b"] == 3
+
+    def test_excluding_inherited_psets(self):
+        element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall")
+        type_element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWallType")
+        ifcopenshell.api.run("type.assign_type", self.file, related_object=element, relating_type=type_element)
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=type_element, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"a": 1, "x": 1})
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=element, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"a": 2, "b": 3})
+        result = subject.get_pset(element, "name", should_inherit=False)
+        assert result["id"] == pset.id()
+        assert result["a"] == 2
+        assert result["b"] == 3
+        assert "x" not in result
+
+    def test_getting_the_psets_of_a_material_as_a_dictionary(self):
+        material = self.file.createIfcMaterial()
+        assert subject.get_psets(material) == {}
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=material, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"x": "y"})
+        assert subject.get_pset(material, "name") == {"x": "y", "id": pset.id()}
+
+    def test_getting_the_psets_of_a_profile_as_a_dictionary(self):
+        profile = self.file.createIfcCircleProfileDef()
+        assert subject.get_psets(profile) == {}
+        pset = ifcopenshell.api.run("pset.add_pset", self.file, product=profile, name="name")
+        ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"x": "y"})
+        assert subject.get_pset(profile, "name") == {"x": "y", "id": pset.id()}
+
+    def test_getting_psets_from_an_element_which_cannot_have_psets(self):
+        assert subject.get_pset(self.file.create_entity("IfcPerson"), "name") == {}
+
+
 class TestGetPsetsIFC4(test.bootstrap.IFC4):
     def test_getting_the_psets_of_a_product_as_a_dictionary(self):
         element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall")