From 490fa92dc02e2feb275d9f3645d95bcfbae1356f Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Thu, 15 Jan 2026 12:37:12 +0100 Subject: [PATCH] Rework equality and get_info_2 --- .../ifcopenshell/entity_instance.py | 3 +- .../test/test_entity_instance.py | 9 ++ src/ifcwrap/IfcParseWrapper.i | 139 ++++++++++++------ 3 files changed, 107 insertions(+), 44 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/entity_instance.py b/src/ifcopenshell-python/ifcopenshell/entity_instance.py index 8e7047d72b..6fe24baf15 100644 --- a/src/ifcopenshell-python/ifcopenshell/entity_instance.py +++ b/src/ifcopenshell-python/ifcopenshell/entity_instance.py @@ -394,7 +394,6 @@ class entity_instance_mixin: * ignore: `()` (empty tuple) """ - assert recursive assert return_type is dict assert len(ignore) == 0 - return ifcopenshell_wrapper.get_info_cpp(self, include_identifier) + return ifcopenshell_wrapper.get_info_cpp(self, recursive, include_identifier) diff --git a/src/ifcopenshell-python/test/test_entity_instance.py b/src/ifcopenshell-python/test/test_entity_instance.py index 6af57585d0..f8a1b773e1 100644 --- a/src/ifcopenshell-python/test/test_entity_instance.py +++ b/src/ifcopenshell-python/test/test_entity_instance.py @@ -67,3 +67,12 @@ class TestGetInfo2(test.bootstrap.IFC4): "Outer": {"CfsFaces": None, "type": "IfcClosedShell"}, "type": "IfcFacetedBrep", } + +def test_equality(): + f = ifcopenshell.file() + g = ifcopenshell.file() + f.createIfcCartesianPoint((0., 0.)) + g.createIfcCartesianPoint((0., 0.)) + assert f[1] == g[1] + g[1].Coordinates = (1., 0.) + assert f[1] != g[1] diff --git a/src/ifcwrap/IfcParseWrapper.i b/src/ifcwrap/IfcParseWrapper.i index fb5b1b479d..95ef423890 100644 --- a/src/ifcwrap/IfcParseWrapper.i +++ b/src/ifcwrap/IfcParseWrapper.i @@ -66,6 +66,7 @@ %ignore IfcParse::IfcFile::type_iterator; %ignore express::Base::is; +%ignore express::Base::operator==; %rename("by_id") instance_by_id; %rename("by_guid") instance_by_guid; @@ -466,29 +467,61 @@ private: } bool __eq__(const express::Base& other) const { - return $self->identity() == other.identity(); + // This logic is not perfect and is not fully consistent with __hash__ + // Should there also be a distinction between entity instances and type declarations? + if ($self->identity() == other.identity()) { + return true; + } else if ($self->file() != other.file()) { + // PyGILState_STATE gil = PyGILState_Ensure(); + // @todo get_info_2 is actually implemented in C++, so we try to call it directly + bool result = false; + PyObject *py_self = SWIG_NewPointerObj(SWIG_as_voidptr($self), + SWIGTYPE_p_express__Base, 0); + PyObject *py_other = SWIG_NewPointerObj(SWIG_as_voidptr(&other), + SWIGTYPE_p_express__Base, 0); + if (py_self && py_other) { + PyObject *args = PyTuple_New(0); + + PyObject *kwargs = PyDict_New(); + PyDict_SetItemString(kwargs, "recursive", Py_True); + PyDict_SetItemString(kwargs, "include_identifier", Py_False); + + PyObject *m1 = PyObject_GetAttrString(py_self, "get_info_2"); + PyObject *m2 = PyObject_GetAttrString(py_other, "get_info_2"); + + PyObject *i1 = (m1 ? PyObject_Call(m1, args, kwargs) : nullptr); + PyObject *i2 = (m2 ? PyObject_Call(m2, args, kwargs) : nullptr); + + if (i1 && i2) { + int eq = PyObject_RichCompareBool(i1, i2, Py_EQ); + result = (eq == 1); + } else { + // get_info_2 missing or threw; treat as not equal (and clear Python error). + PyErr_Clear(); + } + + Py_XDECREF(i1); + Py_XDECREF(i2); + Py_XDECREF(m1); + Py_XDECREF(m2); + Py_DECREF(kwargs); + Py_DECREF(args); + } else { + PyErr_Clear(); + } + + Py_XDECREF(py_self); + Py_XDECREF(py_other); + // PyGILState_Release(gil); + return result; + } else { + return false; + } } size_t __hash__() const { - return boost::hash>{}({self->identity(), self->file()}); - // if (self->declaration().as_entity()) { - // } + return std::hash{}(self->identity()); } - - /* - else { - return self->get_attribute_value(0).apply_visitor([&](const auto& val){ - using U = std::decay_t; - if constexpr (std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v) { - // @todo - return boost::hash>{}({self->declaration().index_in_schema(), 0, self->file()}); - } else { - return boost::hash>{}({self->declaration().index_in_schema(), val, self->file()}); - } - }); - } - } - */ std::string __repr__() const { std::ostringstream oss; @@ -1104,16 +1137,14 @@ object = _old_object %} %{ - PyObject* get_info_cpp(const express::Base& v, bool include_identifier); + PyObject* get_info_cpp(const express::Base& v, bool recursive, bool include_identifier); // @todo refactor this to remove duplication with the typemap. // except this is calls the above function in case of instances. - PyObject* convert_cpp_attribute_to_python(const express::Base& instance, size_t attribute_index, bool include_identifier = true) { - return instance.get_attribute_value(attribute_index).apply_visitor([include_identifier](const auto& v){ + PyObject* convert_cpp_attribute_to_python(const express::Base& instance, size_t attribute_index, bool recursive, bool include_identifier) { + return instance.get_attribute_value(attribute_index).apply_visitor([recursive, include_identifier](const auto& v){ using U = std::decay_t; - if constexpr (is_std_vector_v) { - return pythonize_vector(v); - } else if constexpr (std::is_same_v) { + if constexpr (std::is_same_v) { return pythonize(std::string(v.value())); } else if constexpr (std::is_same_v) { if (feature_use_attribute_value_derived) { @@ -1123,10 +1154,44 @@ object = _old_object return static_cast(Py_None); } } else if constexpr (std::is_same_v) { - return get_info_cpp(v, include_identifier); + if (recursive) { + return get_info_cpp(v, recursive, include_identifier); + } else { + return pythonize(v); + } + } else if constexpr (std::is_same_v>) { + if (recursive) { + PyObject* t = PyTuple_New(v.size()); + for (size_t i = 0; i < v.size(); ++i) { + PyObject* item = get_info_cpp(v[i], recursive, include_identifier); + PyTuple_SET_ITEM(t, i, item); + } + return t; + } else { + return pythonize_vector(v); + } + } else if constexpr (std::is_same_v>>) { + if (recursive) { + PyObject* outer = PyTuple_New(v.size()); + for (size_t i = 0; i < v.size(); ++i) { + const auto& inner_vec = v[i]; + PyObject* inner = PyTuple_New(inner_vec.size()); + for (size_t j = 0; j < inner_vec.size(); ++j) { + PyObject* item = get_info_cpp(inner_vec[j], recursive, include_identifier); + PyTuple_SET_ITEM(inner, j, item); + } + PyTuple_SET_ITEM(outer, i, inner); + } + return outer; + } else { + return pythonize_vector(v); + } } else if constexpr (std::is_same_v || std::is_same_v || std::is_same_v) { Py_INCREF(Py_None); return static_cast(Py_None); + } else if constexpr (is_std_vector_v) { + // only for non-entity-instance vectors + return pythonize_vector(v); } else { return pythonize(v); } @@ -1134,7 +1199,7 @@ object = _old_object } %} %inline %{ - PyObject* get_info_cpp(const express::Base& v, bool include_identifier = true) { + PyObject* get_info_cpp(const express::Base& v, bool recursive, bool include_identifier) { PyObject *d = PyDict_New(); if (v.declaration().as_entity()) { @@ -1147,35 +1212,25 @@ object = _old_object auto attr_type = *dit ? IfcUtil::Argument_DERIVED : IfcUtil::from_parameter_type((*it)->type_of_attribute()); - auto value_py = convert_cpp_attribute_to_python(v, std::distance(attrs.begin(), it), include_identifier); + auto value_py = convert_cpp_attribute_to_python(v, std::distance(attrs.begin(), it), recursive, include_identifier); PyDict_SetItem(d, name_py, value_py); Py_DECREF(name_py); Py_DECREF(value_py); } if (include_identifier) { - const std::string& id_cpp = "id"; - auto id_py = pythonize(id_cpp); auto id_v_py = pythonize(v.id()); - PyDict_SetItem(d, id_py, id_v_py); - Py_DECREF(id_py); + PyDict_SetItemString(d, "id", id_v_py); Py_DECREF(id_v_py); } } else { - const std::string& name_cpp = "wrappedValue"; - auto name_py = pythonize(name_cpp); - auto value_py = convert_cpp_attribute_to_python(v, 0, include_identifier); - PyDict_SetItem(d, name_py, value_py); - Py_DECREF(name_py); + auto value_py = convert_cpp_attribute_to_python(v, 0, recursive, include_identifier); + PyDict_SetItemString(d, "wrappedValue", value_py); Py_DECREF(value_py); } - // @todo type and id can be static? - const std::string& type_cpp = "type"; - auto type_py = pythonize(type_cpp); const std::string& type_v_cpp = v.declaration().name(); auto type_v_py = pythonize(type_v_cpp); - PyDict_SetItem(d, type_py, type_v_py); - Py_DECREF(type_py); + PyDict_SetItemString(d, "type", type_v_py); Py_DECREF(type_v_py); return d;