Add per-property/quantity Unit-override support to edit_pset/edit_qto, plus unit-scale and candidate-unit helpers

edit_pset()'s unpack_unit_value() couldn't distinguish "no Unit dict was
passed" from "{"Unit": None, ...} passed to explicitly clear an existing
override" -- both collapsed to a bare None, and every consuming call site
checked truthiness, so there was no way to actually clear a previously-set
property Unit override once one existed. Fixed with a private _NO_UNIT
sentinel; bare (unwrapped) values still leave Unit untouched exactly as
before.

edit_qto() had no Unit-handling capability at all: neither
update_existing_property() nor add_new_properties() ever read or wrote a
quantity's Unit attribute. Added the same {"Unit": ..., "NominalValue": ...}
wrapped-dict convention edit_pset() already supports, disambiguated from
the pre-existing IfcPhysicalComplexQuantity dict convention
({"Discrimination": ..., "HasQuantities": ...}) by checking for a "Unit"
key -- a complex-quantity spec never contains one.

ifcopenshell.util.unit gains two small helpers:
- get_unit_scale(unit): dispatches to get_derived_unit_scale/
  get_named_unit_scale depending on unit type, also used to de-duplicate
  calculate_unit_scale()'s own inline dispatch of the same logic.
- get_candidate_units(ifc_file, unit_type): all units in a file matching a
  given unit type, unlike get_project_unit()'s single-default lookup.

Adds regression tests for all of the above, including explicit-clear,
bare-value-preserves-override, and complex-quantity-routing-unaffected
cases.
This commit is contained in:
Richard Brice
2026-08-10 10:51:59 -07:00
parent b8211143b1
commit 3a3c00e6f6
6 changed files with 265 additions and 21 deletions
@@ -23,6 +23,10 @@ import ifcopenshell
import ifcopenshell.util.element
import ifcopenshell.util.pset
# Sentinel distinguishing "no Unit dict was passed at all" from "a Unit dict was passed
# with Unit explicitly set to None" (i.e. explicitly clear an existing override).
_NO_UNIT = object()
def edit_pset(
file: ifcopenshell.file,
@@ -285,7 +289,7 @@ class Usecase:
f'Value "{self.settings["properties"][prop.Name]}" is not a valid value for enum property {prop.Name}.'
)
if unit:
if unit is not _NO_UNIT:
prop.Unit = unit
del self.settings["properties"][prop.Name]
return prop
@@ -310,7 +314,7 @@ class Usecase:
)
value = self.cast_value_to_primary_measure_type(value, primary_measure_type)
prop.NominalValue = self.file.create_entity(primary_measure_type, value)
if unit:
if unit is not _NO_UNIT:
prop.Unit = unit
del self.settings["properties"][prop.Name]
return prop
@@ -329,7 +333,7 @@ class Usecase:
# If it's not an entity, then it's a primitive data type
elif not value.is_entity():
kwargs = {"Name": name, "NominalValue": value}
if unit:
if unit is not None and unit is not _NO_UNIT:
kwargs["Unit"] = unit
properties.append(self.file.create_entity("IfcPropertySingleValue", **kwargs))
@@ -353,7 +357,7 @@ class Usecase:
"IfcPropertyListValue",
Name=name,
ListValues=[self.file.create_entity(ifc_class, v) for v in value],
Unit=unit,
Unit=unit if (unit is not None and unit is not _NO_UNIT) else None,
)
)
break
@@ -363,7 +367,7 @@ class Usecase:
"IFCPROPERTYENUMERATION",
Name=name,
EnumerationValues=pset_template.Enumerators.EnumerationValues,
**({"Unit": unit} if unit else {}),
**({"Unit": unit} if (unit is not None and unit is not _NO_UNIT) else {}),
)
prop_enum_value = self.file.create_entity(
"IFCPROPERTYENUMERATEDVALUE",
@@ -389,7 +393,7 @@ class Usecase:
value = self.cast_value_to_primary_measure_type(value, primary_measure_type)
nominal_value = self.file.create_entity(primary_measure_type, value)
args = {"Name": name, "NominalValue": nominal_value}
if unit:
if unit is not None and unit is not _NO_UNIT:
args["Unit"] = unit
properties.append(self.file.create_entity("IfcPropertySingleValue", **args))
@@ -479,12 +483,16 @@ class Usecase:
def unpack_unit_value(value_candidate):
"""
Returns tuple of the format: (Unit, NominalValue)
NOTE: Unit fallbacks to None
NOTE: Unit is the module-level _NO_UNIT sentinel when no Unit was specified at all
(bare value, or a dict without a "Unit" key), so that callers can distinguish "leave
the existing Unit untouched" from an explicit `{"Unit": None, ...}` (clear the
existing Unit override, falling back to the project default).
"""
if value_candidate is None:
return (None, None)
if isinstance(value_candidate, dict): # Custom IfcUnits can be passed in a dict along with the pset value
return (value_candidate["Unit"], value_candidate["NominalValue"])
return (value_candidate.get("Unit", _NO_UNIT), value_candidate["NominalValue"])
return (None, value_candidate)
return (_NO_UNIT, value_candidate)
@@ -136,10 +136,32 @@ def edit_qto(
return usecase.execute()
# Sentinel distinguishing "no Unit dict was passed at all" from "a Unit dict was passed
# with Unit explicitly set to None" (i.e. explicitly clear an existing override).
_NO_UNIT = object()
class Usecase:
file: ifcopenshell.file
settings: dict[str, Any]
@staticmethod
def unpack_unit_value(value_candidate):
"""
Returns tuple of the format: (Unit, NominalValue)
NOTE: a dict value_candidate is ambiguous with the IfcPhysicalComplexQuantity spec
convention ({"Discrimination": ..., "HasQuantities": ...}) used elsewhere in this
module -- callers must check for that case (absence of a "Unit" key) before calling
this. Unit is the module-level _NO_UNIT sentinel when no Unit was specified at all
(bare value, or a dict without a "Unit" key), so that callers can distinguish "leave
the existing Unit untouched" from an explicit `{"Unit": None, ...}` (clear the
existing Unit override, falling back to the project default).
"""
if isinstance(value_candidate, dict) and "Unit" in value_candidate:
return (value_candidate["Unit"], value_candidate["NominalValue"])
return (_NO_UNIT, value_candidate)
def execute(self):
self.qto_idx = 5
if self.settings["qto"].is_a("IfcPhysicalComplexQuantity"):
@@ -173,16 +195,19 @@ class Usecase:
name = prop.Name
if value is None:
self.file.remove(prop)
elif prop.is_a("IfcPhysicalComplexQuantity") and isinstance(value, dict):
elif prop.is_a("IfcPhysicalComplexQuantity") and isinstance(value, dict) and "Unit" not in value:
prop.Discrimination = value.get("Discrimination", prop.Discrimination)
ifcopenshell.api.pset.edit_qto(self.file, qto=prop, properties=value["HasQuantities"])
elif prop.is_a("IfcPhysicalSimpleQuantity"):
unit, value = self.unpack_unit_value(value)
value = value.wrappedValue if isinstance(value, ifcopenshell.entity_instance) else value
# 3 IfcPhysicalSimpleQuantity.XXXValue
if self.file.schema == "IFC4X3" and prop.is_a("IfcQuantityCount"):
prop[3] = int(value)
else:
prop[3] = float(value)
if unit is not _NO_UNIT:
prop.Unit = unit
del self.settings["properties"][name]
def add_new_properties(self) -> list[ifcopenshell.entity_instance]:
@@ -190,21 +215,20 @@ class Usecase:
for name, value in self.settings["properties"].items():
if value is None:
continue
if isinstance(value, dict):
if isinstance(value, dict) and "Unit" not in value:
complex_qto = self.file.create_entity(
"IfcPhysicalComplexQuantity", Name=name, Discrimination=value["Discrimination"]
)
properties.append(complex_qto)
ifcopenshell.api.pset.edit_qto(self.file, qto=complex_qto, properties=value["HasQuantities"])
else:
unit, value = self.unpack_unit_value(value)
property_type = self.get_canonical_property_type(name, value)
value = value.wrappedValue if isinstance(value, ifcopenshell.entity_instance) else value
properties.append(
self.file.create_entity(
"IfcQuantity{}".format(property_type),
**{"Name": name, "{}Value".format(property_type): value},
)
)
kwargs = {"Name": name, "{}Value".format(property_type): value}
if unit is not None and unit is not _NO_UNIT:
kwargs["Unit"] = unit
properties.append(self.file.create_entity("IfcQuantity{}".format(property_type), **kwargs))
return properties
def extend_qto_with_new_properties(self, new_properties: list[ifcopenshell.entity_instance]) -> None:
@@ -498,6 +498,32 @@ def get_project_unit(
return unit
def get_candidate_units(ifc_file: ifcopenshell.file, unit_type: str) -> list[ifcopenshell.entity_instance]:
"""Get all units in the file usable as an override for `unit_type`.
Unlike :func:`get_project_unit`, this returns every matching unit defined
in the file (e.g. both an mm and an m IfcSIUnit might be present), not
just the one currently assigned as the project default.
IfcDerivedUnit is matched first by a literal `UnitType` match, then, as a
fallback, by dimensional analysis (:func:`identify_unit_dimensions`) --
that fallback only helps for the dimension families covered by
`named_dimensions` (the core `IfcUnitEnum` types); it won't match e.g.
`"MODULUSOFELASTICITYUNIT"` by dimension alone, only by literal `UnitType`.
:param ifc_file: The IFC file.
:param unit_type: The type of unit, taken from the list of IFC unit
types, such as "LENGTHUNIT", or an IfcDerivedUnitEnum value such as
"MODULUSOFELASTICITYUNIT".
:return: All matching IfcNamedUnit / IfcDerivedUnit entities in the file.
"""
candidates = [u for u in ifc_file.by_type("IfcNamedUnit") if getattr(u, "UnitType", None) == unit_type]
for unit in ifc_file.by_type("IfcDerivedUnit"):
if getattr(unit, "UnitType", None) == unit_type or identify_unit_dimensions(unit) == unit_type:
candidates.append(unit)
return candidates
def get_property_unit(
prop: ifcopenshell.entity_instance, ifc_file: Union[ifcopenshell.file, None], use_cache: bool = False
) -> Union[ifcopenshell.entity_instance, None]:
@@ -803,6 +829,21 @@ def get_derived_unit_scale(unit: ifcopenshell.entity_instance) -> float:
return scale
def get_unit_scale(unit: ifcopenshell.entity_instance) -> float:
"""Get the scale factor to convert a value in `unit` to SI units.
Dispatches to :func:`get_derived_unit_scale` for IfcDerivedUnit, or
:func:`get_named_unit_scale` otherwise (IfcSIUnit / IfcConversionBasedUnit,
including chains).
:param unit: The unit to get the scale factor for.
:returns: The scale factor.
"""
if unit.is_a("IfcDerivedUnit"):
return get_derived_unit_scale(unit)
return get_named_unit_scale(unit)
def calculate_unit_scale(ifc_file: ifcopenshell.file, unit_type: str = "LENGTHUNIT") -> float:
"""Returns a unit scale factor to convert to and from IFC project units and SI units.
@@ -834,10 +875,7 @@ def calculate_unit_scale(ifc_file: ifcopenshell.file, unit_type: str = "LENGTHUN
for unit in units.Units:
if getattr(unit, "UnitType", ...) != unit_type:
continue
if unit.is_a("IfcDerivedUnit"):
unit_scale *= get_derived_unit_scale(unit)
else:
unit_scale *= get_named_unit_scale(unit)
unit_scale *= get_unit_scale(unit)
return unit_scale
@@ -188,6 +188,36 @@ class TestEditPsetIFC2X3(test.bootstrap.IFC2X3):
assert unit.Prefix == "GIGA"
assert unit.Name == "PASCAL"
def test_explicitly_clearing_a_propertys_unit_override(self):
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
custom_unit = self.file.createIfcSIUnit(UnitType="PRESSUREUNIT", Prefix="GIGA", Name="PASCAL")
pset = ifcopenshell.api.pset.add_pset(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_pset(
self.file, pset=pset, properties={"MyCustom": {"NominalValue": 30.0, "Unit": custom_unit}}
)
prop = pset.HasProperties[0]
assert prop.Unit == custom_unit
ifcopenshell.api.pset.edit_pset(
self.file, pset=pset, properties={"MyCustom": {"NominalValue": 40.0, "Unit": None}}
)
assert prop.Unit is None
assert prop.NominalValue.wrappedValue == 40.0
def test_a_bare_value_does_not_disturb_an_existing_units_override(self):
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
custom_unit = self.file.createIfcSIUnit(UnitType="PRESSUREUNIT", Prefix="GIGA", Name="PASCAL")
pset = ifcopenshell.api.pset.add_pset(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_pset(
self.file, pset=pset, properties={"MyCustom": {"NominalValue": 30.0, "Unit": custom_unit}}
)
prop = pset.HasProperties[0]
assert prop.Unit == custom_unit
ifcopenshell.api.pset.edit_pset(self.file, pset=pset, properties={"MyCustom": 42.0})
assert prop.Unit == custom_unit
assert prop.NominalValue.wrappedValue == 42.0
def test_editing_properties_of_non_rooted_elements(self):
element = self.file.createIfcMaterial()
pset = ifcopenshell.api.pset.add_pset(self.file, product=element, name="Foo_Bar")
@@ -18,6 +18,7 @@
import ifcopenshell.api.pset
import ifcopenshell.api.root
import ifcopenshell.api.unit
import test.bootstrap
@@ -150,3 +151,87 @@ class TestEditQto(test.bootstrap.IFC4):
qto = element.IsDefinedBy[0].RelatingPropertyDefinition
assert qto.Quantities[0].Name == "MyLength"
assert qto.Quantities[0].LengthValue == 34
def test_adding_a_new_quantity_with_a_custom_unit(self):
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
custom_unit = self.file.createIfcSIUnit(UnitType="LENGTHUNIT", Prefix="MILLI", Name="METRE")
qto = ifcopenshell.api.pset.add_qto(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_qto(
self.file, qto=qto, properties={"MyLength": {"NominalValue": 30.0, "Unit": custom_unit}}
)
qto = element.IsDefinedBy[0].RelatingPropertyDefinition
assert qto.Quantities[0].Name == "MyLength"
assert qto.Quantities[0].LengthValue == 30.0
assert qto.Quantities[0].Unit == custom_unit
def test_editing_an_existing_quantitys_unit(self):
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
custom_unit = self.file.createIfcSIUnit(UnitType="LENGTHUNIT", Prefix="MILLI", Name="METRE")
qto = ifcopenshell.api.pset.add_qto(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_qto(self.file, qto=qto, properties={"MyLength": 12.0})
quantity = qto.Quantities[0]
assert quantity.Unit is None
ifcopenshell.api.pset.edit_qto(
self.file, qto=qto, properties={"MyLength": {"NominalValue": 30.0, "Unit": custom_unit}}
)
assert quantity.Unit == custom_unit
assert quantity.LengthValue == 30.0
def test_explicitly_clearing_a_quantitys_unit_override(self):
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
custom_unit = self.file.createIfcSIUnit(UnitType="LENGTHUNIT", Prefix="MILLI", Name="METRE")
qto = ifcopenshell.api.pset.add_qto(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_qto(
self.file, qto=qto, properties={"MyLength": {"NominalValue": 30.0, "Unit": custom_unit}}
)
quantity = qto.Quantities[0]
assert quantity.Unit == custom_unit
ifcopenshell.api.pset.edit_qto(
self.file, qto=qto, properties={"MyLength": {"NominalValue": 40.0, "Unit": None}}
)
assert quantity.Unit is None
assert quantity.LengthValue == 40.0
def test_a_bare_value_does_not_disturb_an_existing_units_override(self):
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
custom_unit = self.file.createIfcSIUnit(UnitType="LENGTHUNIT", Prefix="MILLI", Name="METRE")
qto = ifcopenshell.api.pset.add_qto(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_qto(
self.file, qto=qto, properties={"MyLength": {"NominalValue": 30.0, "Unit": custom_unit}}
)
quantity = qto.Quantities[0]
assert quantity.Unit == custom_unit
ifcopenshell.api.pset.edit_qto(self.file, qto=qto, properties={"MyLength": 42.0})
assert quantity.Unit == custom_unit
assert quantity.LengthValue == 42.0
def test_complex_quantity_editing_is_unaffected_by_the_unit_wrapper_convention(self):
# Regression guard: dict values are already overloaded to mean "this is an
# IfcPhysicalComplexQuantity spec" ({"Discrimination": ..., "HasQuantities": ...}).
# The new {"Unit": ..., "NominalValue": ...} convention must not be confused with it.
element = ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcWall")
qto = ifcopenshell.api.pset.add_qto(self.file, product=element, name="Foo_Bar")
ifcopenshell.api.pset.edit_qto(
self.file,
qto=qto,
properties={"Layers": {"Discrimination": "layer", "HasQuantities": {"Width": 5.0}}},
)
qto = element.IsDefinedBy[0].RelatingPropertyDefinition
complex_qty = qto.Quantities[0]
assert complex_qty.is_a("IfcPhysicalComplexQuantity")
assert complex_qty.Name == "Layers"
assert complex_qty.Discrimination == "layer"
assert complex_qty.HasQuantities[0].Name == "Width"
assert complex_qty.HasQuantities[0].LengthValue == 5.0
# Editing it again (update_existing_property's complex-quantity branch) still works too.
ifcopenshell.api.pset.edit_qto(
self.file,
qto=qto,
properties={"Layers": {"Discrimination": "layer2", "HasQuantities": {"Width": 6.0}}},
)
assert complex_qty.Discrimination == "layer2"
assert complex_qty.HasQuantities[0].LengthValue == 6.0
@@ -95,6 +95,49 @@ class TestGetProjectUnit(test.bootstrap.IFC4):
assert self.file.units == {"LENGTHUNIT": length2, "AREAUNIT": area}
class TestGetCandidateUnits(test.bootstrap.IFC4):
def test_returns_only_units_matching_the_unit_type(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
mm = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT", prefix="MILLI")
m = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT")
area = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="AREAUNIT")
candidates = subject.get_candidate_units(self.file, "LENGTHUNIT")
assert set(candidates) == {mm, m}
assert area not in candidates
def test_returns_all_matching_units_not_just_the_assigned_default(self):
# Unlike get_project_unit, which only returns the one assigned default.
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
mm = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT", prefix="MILLI")
m = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT")
ifcopenshell.api.unit.assign_unit(self.file, units=[mm])
assert subject.get_project_unit(self.file, "LENGTHUNIT") == mm
assert set(subject.get_candidate_units(self.file, "LENGTHUNIT")) == {mm, m}
def test_derived_unit_matched_by_literal_unit_type(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
force = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="FORCEUNIT")
area = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="AREAUNIT")
modulus = ifcopenshell.api.unit.add_derived_unit(self.file, "MODULUSOFELASTICITYUNIT", None, {force: 1, area: -1})
assert subject.get_candidate_units(self.file, "MODULUSOFELASTICITYUNIT") == [modulus]
def test_userdefined_derived_unit_matched_by_dimensional_fallback(self):
# No literal UnitType match (USERDEFINED), but dimensionally it's a pressure unit,
# and PRESSUREUNIT is one of the core families covered by named_dimensions.
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
force = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="FORCEUNIT")
area = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="AREAUNIT")
weird_pressure = ifcopenshell.api.unit.add_derived_unit(
self.file, "USERDEFINED", "pressure-ish", {force: 1, area: -1}
)
assert subject.get_candidate_units(self.file, "PRESSUREUNIT") == [weird_pressure]
def test_empty_when_nothing_matches(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT")
assert subject.get_candidate_units(self.file, "MASSUNIT") == []
class TestGetPropertyUnit(test.bootstrap.IFC4):
def test_no_unit(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
@@ -302,6 +345,22 @@ class TestGetDerivedUnitScale(test.bootstrap.IFC4):
assert subject.get_derived_unit_scale(weird) == pytest.approx(1 / 0.001)
class TestGetUnitScale(test.bootstrap.IFC4):
def test_dispatches_to_named_unit_scale_for_si_and_conversion_based_units(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
mm = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT", prefix="MILLI")
ft = ifcopenshell.api.unit.add_conversion_based_unit(self.file, name="foot")
assert subject.get_unit_scale(mm) == subject.get_named_unit_scale(mm)
assert subject.get_unit_scale(ft) == subject.get_named_unit_scale(ft)
def test_dispatches_to_derived_unit_scale_for_derived_units(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
mass = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="MASSUNIT", prefix="KILO")
volume = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="VOLUMEUNIT")
density = ifcopenshell.api.unit.add_derived_unit(self.file, "MASSDENSITYUNIT", None, {mass: 1, volume: -1})
assert subject.get_unit_scale(density) == subject.get_derived_unit_scale(density)
class TestGetUnitSymbol(test.bootstrap.IFC4):
def test_derived_unit_composes_a_symbol(self):
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")