mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-21 14:23:53 +00:00
sql, stream: raise for accessing derived attributes
Raising `AttributeError` is definitely wasn't correct here, since it might push the code to assume it's a wrong entity type. Returning some stub value like `None` also could suggest incorrect derived attribute value, leading to unexpected behaviour. So adding an error, so it would propagate and code would need to be adjusted not to rely on derived attributes, if it actually interacts with sql/stream. `test_unit` was asserting that derived attr will return `None`, though it was actually raising `AttributeError`.
This commit is contained in:
@@ -380,9 +380,9 @@ class sqlite_entity:
|
|||||||
INVALID, FORWARD, INVERSE, DERIVED = range(4)
|
INVALID, FORWARD, INVERSE, DERIVED = range(4)
|
||||||
attr_cat = self.wrapped_data.get_attribute_category(name)
|
attr_cat = self.wrapped_data.get_attribute_category(name)
|
||||||
if attr_cat == DERIVED:
|
if attr_cat == DERIVED:
|
||||||
# Derived attributes aren't stored or computed for SQLite-linked
|
raise RuntimeError(
|
||||||
# files, so callers must treat None as "not available" here.
|
f"Derived attributes (e.g. '{name}') are not supported for {type(self).__name__} entities."
|
||||||
return None
|
)
|
||||||
if attr_cat == FORWARD:
|
if attr_cat == FORWARD:
|
||||||
if self.sqlite_wrapper.attribute_cache:
|
if self.sqlite_wrapper.attribute_cache:
|
||||||
return self.sqlite_wrapper.attribute_cache[name]
|
return self.sqlite_wrapper.attribute_cache[name]
|
||||||
|
|||||||
@@ -360,8 +360,12 @@ try:
|
|||||||
assert False, "Not supported during streaming."
|
assert False, "Not supported during streaming."
|
||||||
|
|
||||||
def __getattr__(self, name: str) -> Any:
|
def __getattr__(self, name: str) -> Any:
|
||||||
INVALID, FORWARD, INVERSE = range(3)
|
INVALID, FORWARD, INVERSE, DERIVED = range(4)
|
||||||
attr_cat = self.wrapped_data.get_attribute_category(name)
|
attr_cat = self.wrapped_data.get_attribute_category(name)
|
||||||
|
if attr_cat == DERIVED:
|
||||||
|
raise RuntimeError(
|
||||||
|
f"Derived attributes (e.g. '{name}') are not supported for {type(self).__name__} entities."
|
||||||
|
)
|
||||||
if attr_cat == FORWARD:
|
if attr_cat == FORWARD:
|
||||||
if self.stream_wrapper.attribute_cache:
|
if self.stream_wrapper.attribute_cache:
|
||||||
return self.stream_wrapper.attribute_cache[name]
|
return self.stream_wrapper.attribute_cache[name]
|
||||||
|
|||||||
@@ -19,6 +19,7 @@
|
|||||||
import tempfile
|
import tempfile
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
from ifcpatch.recipes import Ifc2Sql
|
from ifcpatch.recipes import Ifc2Sql
|
||||||
|
|
||||||
import ifcopenshell
|
import ifcopenshell
|
||||||
@@ -63,6 +64,12 @@ class TestEntity:
|
|||||||
assert (element := ifc_sqlite.by_id(1))
|
assert (element := ifc_sqlite.by_id(1))
|
||||||
assert element.Name == "My Project"
|
assert element.Name == "My Project"
|
||||||
|
|
||||||
|
def test_getattr_derive(self):
|
||||||
|
ifc_sqlite = get_ifc_sqlite()
|
||||||
|
assert (units := ifc_sqlite.by_type("IfcSIUnit"))
|
||||||
|
with pytest.raises(RuntimeError):
|
||||||
|
units[0].Dimensions
|
||||||
|
|
||||||
def test_setattr(self):
|
def test_setattr(self):
|
||||||
ifc_sqlite = get_ifc_sqlite()
|
ifc_sqlite = get_ifc_sqlite()
|
||||||
assert (element := ifc_sqlite.by_id(1))
|
assert (element := ifc_sqlite.by_id(1))
|
||||||
|
|||||||
@@ -18,6 +18,8 @@
|
|||||||
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
import ifcopenshell
|
import ifcopenshell
|
||||||
|
|
||||||
TEST_FILE = Path(__file__).parent / "files" / "basic.ifc"
|
TEST_FILE = Path(__file__).parent / "files" / "basic.ifc"
|
||||||
@@ -48,3 +50,10 @@ class TestEntity:
|
|||||||
stream_file = ifcopenshell.open(TEST_FILE, should_stream=True)
|
stream_file = ifcopenshell.open(TEST_FILE, should_stream=True)
|
||||||
assert (element := stream_file.by_id(1))
|
assert (element := stream_file.by_id(1))
|
||||||
assert element.Name == "My Project"
|
assert element.Name == "My Project"
|
||||||
|
|
||||||
|
def test_getattr_derive(self):
|
||||||
|
stream_file: ifcopenshell.stream
|
||||||
|
stream_file = ifcopenshell.open(TEST_FILE, should_stream=True)
|
||||||
|
assert (units := stream_file.by_type("IfcSIUnit"))
|
||||||
|
with pytest.raises(RuntimeError):
|
||||||
|
units[0].Dimensions
|
||||||
|
|||||||
@@ -329,11 +329,9 @@ class TestCalculateUnitScaleOnLinkedFile(test.bootstrap.IFC4):
|
|||||||
def test_run(self):
|
def test_run(self):
|
||||||
# Regression test: IfcSIUnit.Dimensions is a schema-*derived*
|
# Regression test: IfcSIUnit.Dimensions is a schema-*derived*
|
||||||
# attribute that isn't computed for SQLite-linked files (used for
|
# attribute that isn't computed for SQLite-linked files (used for
|
||||||
# Bonsai's "linked project" large-model workflow), so it returns None
|
# Bonsai's "linked project" large-model workflow) and raises there
|
||||||
# there instead of an IfcDimensionalExponents entity. calculate_unit_scale()
|
# instead of returning an IfcDimensionalExponents entity.
|
||||||
# used to access unit.Dimensions.LengthExponent unconditionally for
|
# Test ensures `calculate_unit_scale` doesn't rely on derived attribute computation.
|
||||||
# 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")
|
ifcopenshell.api.root.create_entity(self.file, ifc_class="IfcProject")
|
||||||
length = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT")
|
length = ifcopenshell.api.unit.add_si_unit(self.file, unit_type="LENGTHUNIT")
|
||||||
ifcopenshell.api.unit.assign_unit(self.file, units=[length])
|
ifcopenshell.api.unit.assign_unit(self.file, units=[length])
|
||||||
@@ -345,7 +343,6 @@ class TestCalculateUnitScaleOnLinkedFile(test.bootstrap.IFC4):
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
linked_file = ifcopenshell.open(str(tmp_file))
|
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
|
assert subject.calculate_unit_scale(linked_file, "LENGTHUNIT") == 1.0
|
||||||
finally:
|
finally:
|
||||||
if isinstance(linked_file, ifcopenshell.sqlite):
|
if isinstance(linked_file, ifcopenshell.sqlite):
|
||||||
|
|||||||
Reference in New Issue
Block a user