Fix running of test/tests.py

src/ifcopenshell-python/ifcopenshell/__init__.py

@@ -169,6 +169,7 @@ def open(
         print(products[0].id(), products[0].GlobalId) # 122 2XQ$n5SLP5MBLyL442paFx
         print(products[0] == model[122] == model["2XQ$n5SLP5MBLyL442paFx"]) # True
     """
+
     path = Path(path)
     if not path.exists():
         raise FileNotFoundError(f"Path does not exist: '{path}'.")
@@ -205,7 +206,33 @@ def open(
         f = ifcopenshell_wrapper.open(str(path.absolute()), mmap=mmap)
     else:
         f = ifcopenshell_wrapper.open(str(path.absolute()))
-    return file(f)
+
+    f.post_init()
+
+    READ_ERROR = ifcopenshell_wrapper.file_open_status.READ_ERROR
+    NO_HEADER = ifcopenshell_wrapper.file_open_status.NO_HEADER
+    UNSUPPORTED_SCHEMA = ifcopenshell_wrapper.file_open_status.UNSUPPORTED_SCHEMA
+    INVALID_SYNTAX = ifcopenshell_wrapper.file_open_status.INVALID_SYNTAX
+    UNKNOWN = ifcopenshell_wrapper.file_open_status.UNKNOWN
+
+    if not f.good():
+        from . import Error, SchemaError
+
+        exc, msg = {
+            READ_ERROR: lambda: (IOError, "Unable to open file for reading"),
+            NO_HEADER: lambda: (Error, "Unable to parse IFC SPF header"),
+            UNSUPPORTED_SCHEMA: lambda: (
+                SchemaError,
+                "Unsupported schema: %s" % ",".join(f.header.file_schema.schema_identifiers),
+            ),
+            INVALID_SYNTAX: lambda: (Error, "Syntax error during parse, check logs"),
+            # This is the case when passing uninitialized_tag
+            UNKNOWN: lambda: (None, None),
+        }[f.good().value()]()
+        if exc is not None:
+            raise exc(msg)
+
+    return f
 
 
 def create_entity(type: str, schema: str = "IFC4", *args: Any, **kwargs: Any) -> entity_instance:

src/ifcopenshell-python/ifcopenshell/express/templates.py

@@ -270,7 +270,7 @@ get_attr_stmt_entity = "%(null_check)s return ((express::Base)(get_attribute_val
 get_attr_stmt_array = "%(null_check)s std::vector<express::Base> es = get_attribute_value(%(index)d); return cast_vector<%(list_instance_type)s>(es);"
 get_attr_stmt_nested_array = "%(null_check)s std::vector<std::vector<express::Base>> es = get_attribute_value(%(index)d); return cast_vector_vector<%(list_instance_type)s>(es);"
 
-get_inverse = "return cast_vector<%(type)s>(data()->file()->getInverse(data()->id(), %(schema_name_upper)s_types[%(type_index)d], %(index)d));"
+get_inverse = "return cast_vector<%(type)s>(file()->getInverse(data()->id(), %(schema_name_upper)s_types[%(type_index)d], %(index)d));"
 
 set_attr_stmt = (
     "%(check_optional_set_begin)sset_attribute_value(%(index)d, %(star_if_optional)sv);%(check_optional_set_else)sunset_attribute_value(%(index)d);%(check_optional_set_end)s"

src/ifcopenshell-python/ifcopenshell/file.py

@@ -20,11 +20,9 @@ from __future__ import annotations
 import os
 import re
 import numbers
-import time
 import zipfile
 import functools
 import ifcopenshell
-import weakref
 import types
 from pathlib import Path
 from typing import Any, Optional, TYPE_CHECKING, Union, overload, Literal, TypedDict
@@ -414,13 +412,13 @@ class rocksdb_file_storage:
         self._prefix = prefix
 
     def items(self):
-        it = self.file.wrapped_data.key_value_store_iter(self._prefix)
+        it = self.file.key_value_store_iter(self._prefix)
         while it and it.valid():
             yield it.key(), it.value()
             it.next()
 
     def read(self, key):
-        return self.file.wrapped_data.key_value_store_query(key)
+        return self.file.key_value_store_query(key)
 
     def by_id(self, name):
         if isinstance(name, tuple):
@@ -480,25 +478,6 @@ class rocksdb_file_storage:
         return "".join("".join(map(str, t)) if t[1] else "" for t in zip(prefixes, version_tuple[0:2]))
 
 
-class file_header:
-    def __init__(self, file, header_data):
-        self.file = file
-        self.header_data = header_data
-
-    # @todo these are probably no longer necessary now as we no longer depend on decoration of file
-    @property
-    def file_description(self) -> ifcopenshell.entity_instance:
-        return self.header_data.file_description_py()
-
-    @property
-    def file_name(self) -> ifcopenshell.entity_instance:
-        return self.header_data.file_name_py()
-
-    @property
-    def file_schema(self) -> ifcopenshell.entity_instance:
-        return self.header_data.file_schema_py()
-
-
 class file_mixin:
     """Base class for containing IFC files.
 
@@ -611,7 +590,7 @@ class file_mixin:
         # @todo we should probably check that values for
         # attributes are not passed as duplicates using
         # both regular arguments and keyword arguments.
-        kwargs_attrs = [(e.wrapped_data.get_argument_index(name), arg) for name, arg in kwargs.items()]
+        kwargs_attrs = [(e.get_argument_index(name), arg) for name, arg in kwargs.items()]
         attrs = list(enumerate(args)) + kwargs_attrs
 
         if len(attrs) > len(e):
@@ -799,11 +778,11 @@ class file_mixin:
         if with_attribute_indices and not allow_duplicate:
             raise ValueError("with_attribute_indices requires allow_duplicate to be True")
 
-        inverses = [entity_instance(e, self) for e in self.get_inverse(inst.wrapped_data)]
+        inverses = self.get_inverse(inst)
 
         if allow_duplicate:
             if with_attribute_indices:
-                idxs = self.get_inverse_indices(inst.wrapped_data)
+                idxs = self.get_inverse_indices(inst)
                 # TODO: include in typing.
                 return list(zip(inverses, idxs))
             else:
@@ -811,17 +790,6 @@ class file_mixin:
 
         return set(inverses)
 
-    def get_total_inverses(self, inst: ifcopenshell.entity_instance) -> int:
-        """Returns the number of entities that reference this entity
-
-        This is equivalent to `len(model.get_inverse(element))`, but
-        significantly faster.
-
-        :param inst: The entity instance to get inverse relationships
-        :returns: The total number of references
-        """
-        return self.get_total_inverses(inst.wrapped_data)
-
     def remove(self, inst: ifcopenshell.entity_instance) -> None:
         """Deletes an IFC object in the file.
 
@@ -833,7 +801,7 @@ class file_mixin:
         """
         if self.transaction:
             self.transaction.store_delete(inst)
-        return self.remove(inst.wrapped_data)
+        return self.remove(inst)
 
     def batch(self):
         """Low-level mechanism to speed up deletion of large subgraphs"""
@@ -904,16 +872,6 @@ class file_mixin:
     def to_string(self) -> str:
         return self.to_string()
 
-    @property
-    def header(self) -> file_header:
-        # TODO: Workaround for old builds, remove after build stabilizes.
-        # TODO: No need for `wrapped_data.header` to be a method - should use `@property`?
-        header = self.header
-        if isinstance(header, types.MethodType):
-            return file_header(self, self.header())
-        else:
-            return self.header
-
     @property
     def storage(self) -> Optional[rocksdb_file_storage]:
         """

src/ifcopenshell-python/ifcopenshell/geom/main.py

@@ -23,8 +23,7 @@ import sys
 import operator
 
 from .. import open, ifcopenshell_wrapper
-from ..file import file
+from ifcopenshell import file, entity_instance
-from ..entity_instance import entity_instance
 
 from . import has_occ
 
@@ -305,7 +304,7 @@ class iterator(ifcopenshell_wrapper.Iterator):
         self.settings = settings
         if isinstance(file_or_filename, file):
             self.file = file
-            file_or_filename = file_or_filename.wrapped_data
+            file_or_filename = file_or_filename
         else:
             file_or_filename = self.file = open(file_or_filename)
 
@@ -367,13 +366,13 @@ class tree(ifcopenshell_wrapper.tree):
     def __init__(self, file: Optional[file] = None, settings: Optional[settings] = None):
         args = [self]
         if file is not None:
-            args.append(file.wrapped_data)
+            args.append(file)
             if settings is not None:
                 args.append(settings)
         ifcopenshell_wrapper.tree.__init__(*args)
 
     def add_file(self, file: file, settings: settings) -> None:
-        ifcopenshell_wrapper.tree.add_file(self, file.wrapped_data, settings)
+        ifcopenshell_wrapper.tree.add_file(self, file, settings)
 
     def add_iterator(self, iterator: iterator) -> None:
         ifcopenshell_wrapper.tree.add_file(self, iterator)
@@ -387,7 +386,7 @@ class tree(ifcopenshell_wrapper.tree):
     ) -> list[entity_instance]:
         def unwrap(value):
             if isinstance(value, entity_instance):
-                return value.wrapped_data
+                return value
             elif all(map(lambda v: hasattr(value, v), "XYZ")):
                 return value.X(), value.Y(), value.Z()
             return value
@@ -406,12 +405,12 @@ class tree(ifcopenshell_wrapper.tree):
                 args.append(kwargs.get("completely_within", False))
                 if "extend" in kwargs:
                     args.append(kwargs["extend"])
-        return [entity_instance(e) for e in ifcopenshell_wrapper.tree.select(*args)]
+        return ifcopenshell_wrapper.tree.select(*args)
 
     def select_box(self, value, **kwargs) -> list[entity_instance]:
         def unwrap(value):
             if isinstance(value, entity_instance):
-                return value.wrapped_data
+                return value
             elif hasattr(value, "Get"):
                 return value.Get()[:3], value.Get()[3:]
             return value
@@ -421,7 +420,7 @@ class tree(ifcopenshell_wrapper.tree):
             args.append(kwargs.get("completely_within", False))
         if "extend" in kwargs:
             args.append(kwargs.get("extend", -1.0e-5))
-        return [entity_instance(e) for e in ifcopenshell_wrapper.tree.select_box(*args)]
+        return ifcopenshell_wrapper.tree.select_box(*args)
 
     def clash_intersection_many(
         self,
@@ -430,13 +429,13 @@ class tree(ifcopenshell_wrapper.tree):
         tolerance: float = 0.002,
         check_all: bool = True,
     ) -> tuple[ifcopenshell_wrapper.clash, ...]:
-        args = [self, [e.wrapped_data for e in set_a], [e.wrapped_data for e in set_b], tolerance, check_all]
+        args = [self, set_a, set_b, tolerance, check_all]
         return ifcopenshell_wrapper.tree.clash_intersection_many(*args)
 
     def clash_collision_many(
         self, set_a: Iterable[entity_instance], set_b: Iterable[entity_instance], allow_touching=False
     ) -> tuple[ifcopenshell_wrapper.clash, ...]:
-        args = [self, [e.wrapped_data for e in set_a], [e.wrapped_data for e in set_b], allow_touching]
+        args = [self, set_a, set_b, allow_touching]
         return ifcopenshell_wrapper.tree.clash_collision_many(*args)
 
     def clash_clearance_many(
@@ -446,7 +445,7 @@ class tree(ifcopenshell_wrapper.tree):
         clearance: float = 0.05,
         check_all: bool = False,
     ) -> tuple[ifcopenshell_wrapper.clash, ...]:
-        args = [self, [e.wrapped_data for e in set_a], [e.wrapped_data for e in set_b], clearance, check_all]
+        args = [self, set_a, set_b, clearance, check_all]
         return ifcopenshell_wrapper.tree.clash_clearance_many(*args)
 
     @staticmethod
@@ -509,7 +508,7 @@ def create_shape(
     return wrap_shape_creation(
         settings,
         ifcopenshell_wrapper.create_shape(
-            settings, inst.wrapped_data, repr.wrapped_data if repr is not None else None, geometry_library
+            settings, inst, repr if repr is not None else None, geometry_library
         ),
     )
 
@@ -525,7 +524,7 @@ def map_shape(settings: settings, inst: entity_instance) -> ifcopenshell_wrapper
     >>> ifcopenshell.geom.map_shape(ifcopenshell.geom.settings(), point).components
     (0.0, 0.0, 0.0)
     """
-    return ifcopenshell_wrapper.map_shape(settings, inst.wrapped_data)
+    return ifcopenshell_wrapper.map_shape(settings, inst)
 
 
 @overload
@@ -618,20 +617,17 @@ def iterate(
 
 
 def make_shape_function(fn):
-    def entity_instance_or_none(e):
-        return None if e is None else entity_instance(e)
-
     if has_occ:
 
         def _(schema, string_or_shape, *args):
             if isinstance(string_or_shape, TopoDS.TopoDS_Shape):
                 string_or_shape = utils.serialize_shape(string_or_shape)
-            return entity_instance_or_none(fn(schema, string_or_shape, *args))
+            return fn(schema, string_or_shape, *args)
 
     else:
 
         def _(schema, string, *args):
-            return entity_instance_or_none(fn(schema, string, *args))
+            return fn(schema, string, *args)
 
     return _
 

src/ifcparse/IfcFile.cpp

@@ -774,8 +774,12 @@ express::Base IfcParse::impl::rocks_db_file_storage::create(const IfcParse::decl
 }
 
 express::Base IfcParse::impl::in_memory_file_storage::create(const IfcParse::declaration* decl, int id) {
-    auto instance_name = id == -1 ? (int)file->FreshId() : id;
+    uint32_t instance_name;
-    if (decl->as_entity() == nullptr && decl->as_type_declaration() == nullptr) {
+    if (decl->as_entity() != nullptr) {
+        instance_name = id == -1 ? (int)file->FreshId() : id;
+    } else if (decl->as_type_declaration() != nullptr) {
+        instance_name = 0;
+    } else {
         throw std::runtime_error("Requires and entity or type declaration");
     }
     auto ptr = byid_.insert({instance_name, std::make_shared<InstanceData>(file, decl, instance_name, decl->as_entity() ? in_memory_attribute_storage(decl->as_entity()->attribute_count()) : in_memory_attribute_storage(1))}).first;

src/ifcparse/IfcParse.cpp

@@ -1840,8 +1840,6 @@ express::Base IfcFile::addEntity(const express::Base& entity, int id) {
         return mit->second;
     }
 
-    express::Base new_entity;
-
     // Obtain all forward references by a depth-first
     // traversal and add them to the file.
     try {
@@ -1862,7 +1860,7 @@ express::Base IfcFile::addEntity(const express::Base& entity, int id) {
     // container and entity is created. The attribute references
     // need to be updated to point to instances in this file.
     IfcFile* other_file = entity.file();
-    create(&entity.declaration(), id);
+    auto new_entity = create(&entity.declaration(), id);
     auto* decl = &entity.declaration();
 
     auto num_attributes = (decl->as_entity() ? decl->as_entity()->attribute_count() : 1);

src/ifcwrap/IfcParseWrapper.i

@@ -342,6 +342,7 @@ private:
 
 	%pythoncode %{
 		schema = property(schema_name)
+		header = property(header)
 
 		old_init = __init__
 		def __init__(self, schema=None, schema_version=None):
@@ -519,7 +520,7 @@ private:
 		return IfcUtil::ArgumentTypeToString(helper_fn_attribute_type($self, i));
 	}
 
-	const std::string& get_argument_name(unsigned int i) const {
+	const std::string& attribute_name(unsigned int i) const {
 		if ($self->declaration().as_entity()) {
 			return $self->declaration().as_entity()->attribute_by_index(i)->name();
 		} else if (i == 0) {
@@ -821,31 +822,9 @@ private:
 	}
 }
 
-// Expose FileDescription and FileName header entities
-// to make them readable even if they were not filled properly before.
-// Though it is invalid IFC, technically.
-// FileSchema is not exposed as IFC file won't load if it's invalid.
-
-%extend IfcParse::FileDescription {
-    AttributeValue description() const { return $self->getArgument(0); }
-    AttributeValue implementation_level() const { return $self->getArgument(1); }
-};
-
-%extend IfcParse::FileName {
-    AttributeValue name() const { return $self->getArgument(0); }
-    AttributeValue time_stamp() const { return $self->getArgument(1); }
-    AttributeValue author() const { return $self->getArgument(2); }
-    AttributeValue organization() const { return $self->getArgument(3); }
-    AttributeValue preprocessor_version() const { return $self->getArgument(4); }
-    AttributeValue originating_system() const { return $self->getArgument(5); }
-    AttributeValue authorization() const { return $self->getArgument(6); }
-};
-
 %extend IfcParse::IfcSpfHeader {
-	// Cast to base class pointers for SWIG, because
+	// Upcast to header instances for SWIG, because
 	// it has no idea about the schema definitions.
-	// The code to access these methods as attributes
-	// is in file.py
 	express::Base file_description_py() {
 		return $self->file_description();
 	}
@@ -855,32 +834,11 @@ private:
 	express::Base file_schema_py() {
 		return $self->file_schema();
 	}
-};
 
-%extend IfcParse::FileDescription {
 	%pythoncode %{
-        # Hide the getters with read-write property implementations
+		file_description = property(file_description_py)
-        description = property(description, description)
+		file_name = property(file_name_py)
-        implementation_level = property(implementation_level, implementation_level)
+		file_schema = property(file_schema_py)
-	%}
-};
-
-%extend IfcParse::FileName {
-	%pythoncode %{
-        name = property(name, name)
-        time_stamp = property(time_stamp, time_stamp)
-        author = property(author, author)
-        organization = property(organization, organization)
-        preprocessor_version = property(preprocessor_version, preprocessor_version)
-        originating_system = property(originating_system, originating_system)
-        authorization = property(authorization, authorization)
-	%}
-};
-
-%extend IfcParse::FileSchema {
-	%pythoncode %{
-        # Hide the getters with read-write property implementations
-        schema_identifiers = property(schema_identifiers, schema_identifiers)
 	%}
 };
 

test/tests.py

@@ -53,6 +53,8 @@ f2 = ifcopenshell.file(schema=f.schema)
 prop2 = f2.add(prop)
 assert str(prop) == str(prop2).replace(str(prop2.id()), str(prop.id()))
 assert prop2.id() == 1
+# Adding the same instance returns the previous copy
+assert f2.add(prop) == prop2
 
 # A recursively obtained python dictionary representation
 # matches for copied instances as well