mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-22 01:00:56 +00:00
Fix calculate_unit_scale() crash on SQLite-linked files
IfcSIUnit.Dimensions is a schema-derived attribute that isn't computed for Bonsai's SQLite-linked "large model" file representation, returning None there instead of an IfcDimensionalExponents entity. #9278 added an unconditional unit.Dimensions.LengthExponent access to every IfcSIUnit processed by calculate_unit_scale(), so it crashed project loading for any linked file, even ones with no unit prefixes at all -- not just the prefixed-area/volume case the fix targeted. Fixed by reading dimensions from the existing si_dimensions table (keyed by the unit's stored Name, not the unresolvable derived attribute) instead of unit.Dimensions. See the PR discussion for a standalone reproduction script.
This commit is contained in:
@@ -729,18 +729,13 @@ def calculate_unit_scale(ifc_file: ifcopenshell.file, unit_type: str = "LENGTHUN
|
|||||||
# non-length dimensions (PASCAL, NEWTON, GRAM, ...) keep the linear
|
# non-length dimensions (PASCAL, NEWTON, GRAM, ...) keep the linear
|
||||||
# multiplier, as there the prefix scales the derived unit itself.
|
# multiplier, as there the prefix scales the derived unit itself.
|
||||||
# https://github.com/IfcOpenShell/IfcOpenShell/issues/9278
|
# https://github.com/IfcOpenShell/IfcOpenShell/issues/9278
|
||||||
dimensions = unit.Dimensions
|
#
|
||||||
length_exponent = dimensions.LengthExponent
|
# Dimensions is looked up from si_dimensions by name rather than via
|
||||||
if length_exponent > 0 and not any(
|
# unit.Dimensions (the schema-derived IfcDimensionalExponents), since
|
||||||
(
|
# the derived attribute isn't computed for SQLite-linked files and
|
||||||
dimensions.MassExponent,
|
# would return None there.
|
||||||
dimensions.TimeExponent,
|
length_exponent, *other_exponents = get_si_dimensions(unit.Name.replace("METER", "METRE"))
|
||||||
dimensions.ElectricCurrentExponent,
|
if length_exponent > 0 and not any(other_exponents):
|
||||||
dimensions.ThermodynamicTemperatureExponent,
|
|
||||||
dimensions.AmountOfSubstanceExponent,
|
|
||||||
dimensions.LuminousIntensityExponent,
|
|
||||||
)
|
|
||||||
):
|
|
||||||
prefix_multiplier **= length_exponent
|
prefix_multiplier **= length_exponent
|
||||||
unit_scale *= prefix_multiplier
|
unit_scale *= prefix_multiplier
|
||||||
return unit_scale
|
return unit_scale
|
||||||
|
|||||||
@@ -16,7 +16,9 @@
|
|||||||
# You should have received a copy of the GNU Lesser General Public License
|
# You should have received a copy of the GNU Lesser General Public License
|
||||||
# along with IfcOpenShell. If not, see <http://www.gnu.org/licenses/>.
|
# along with IfcOpenShell. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
|
||||||
|
import tempfile
|
||||||
from math import pi
|
from math import pi
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
import numpy as np
|
import numpy as np
|
||||||
import pytest
|
import pytest
|
||||||
@@ -29,8 +31,10 @@ import ifcopenshell.api.unit
|
|||||||
import ifcopenshell.util.element
|
import ifcopenshell.util.element
|
||||||
import ifcopenshell.util.geolocation
|
import ifcopenshell.util.geolocation
|
||||||
import ifcopenshell.util.unit as subject
|
import ifcopenshell.util.unit as subject
|
||||||
|
import ifcpatch
|
||||||
import test.bootstrap
|
import test.bootstrap
|
||||||
from ifcopenshell.util.shape_builder import ShapeBuilder
|
from ifcopenshell.util.shape_builder import ShapeBuilder
|
||||||
|
from ifcpatch.recipes import Ifc2Sql
|
||||||
|
|
||||||
|
|
||||||
class TestMmToM:
|
class TestMmToM:
|
||||||
@@ -226,6 +230,34 @@ class TestCalculateUnitScale(test.bootstrap.IFC4):
|
|||||||
assert subject.calculate_unit_scale(self.file, "MASSUNIT") == pytest.approx(1000)
|
assert subject.calculate_unit_scale(self.file, "MASSUNIT") == pytest.approx(1000)
|
||||||
|
|
||||||
|
|
||||||
|
class TestCalculateUnitScaleOnLinkedFile(test.bootstrap.IFC4):
|
||||||
|
def test_run(self):
|
||||||
|
# Regression test: IfcSIUnit.Dimensions is a schema-*derived*
|
||||||
|
# attribute that isn't computed for SQLite-linked files (used for
|
||||||
|
# Bonsai's "linked project" large-model workflow), so it returns None
|
||||||
|
# there instead of an IfcDimensionalExponents entity. calculate_unit_scale()
|
||||||
|
# used to access unit.Dimensions.LengthExponent unconditionally for
|
||||||
|
# every IfcSIUnit, which crashed project loading for any linked file.
|
||||||
|
# See the PR discussion for a standalone reproduction script.
|
||||||
|
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
|
||||||
|
length = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT")
|
||||||
|
ifcopenshell.api.unit.assign_unit(self.file, units=[length])
|
||||||
|
|
||||||
|
patcher = Ifc2Sql.Patcher(self.file, sql_type="SQLite")
|
||||||
|
patcher.patch()
|
||||||
|
tmp_file = Path(tempfile.mkstemp(suffix=".ifcsqlite")[1])
|
||||||
|
ifcpatch.write(patcher.get_output(), tmp_file)
|
||||||
|
|
||||||
|
try:
|
||||||
|
linked_file = ifcopenshell.open(str(tmp_file))
|
||||||
|
assert linked_file.by_type("IfcSIUnit")[0].Dimensions is None
|
||||||
|
assert subject.calculate_unit_scale(linked_file, "LENGTHUNIT") == 1.0
|
||||||
|
finally:
|
||||||
|
if isinstance(linked_file, ifcopenshell.sqlite):
|
||||||
|
linked_file.db.close()
|
||||||
|
tmp_file.unlink(missing_ok=True)
|
||||||
|
|
||||||
|
|
||||||
class TestFormatLength(test.bootstrap.IFC4):
|
class TestFormatLength(test.bootstrap.IFC4):
|
||||||
def test_run(self):
|
def test_run(self):
|
||||||
assert subject.format_length(1, 1, decimal_places=0, unit_system="metric") == "1"
|
assert subject.format_length(1, 1, decimal_places=0, unit_system="metric") == "1"
|
||||||
|
|||||||
Reference in New Issue
Block a user