entity_instance: comparison operators to support non-entity types

src/ifcopenshell-python/ifcopenshell/__init__.py

@@ -93,11 +93,6 @@ except Exception:
 from . import guid
 from .ifcopenshell_wrapper import entity_instance, file
 from .file import rocksdb_lazy_instance
-# Hacks!
-from .entity_instance import _patch_swig_comparisons
-_patch_swig_comparisons()
-del _patch_swig_comparisons
-# End hacks!
 from .sql import sqlite, sqlite_entity
 
 get_log = ifcopenshell_wrapper.get_log

src/ifcopenshell-python/ifcopenshell/entity_instance.py

@@ -208,6 +208,7 @@ class entity_instance_mixin:
 
         return value
 
+    # TODO: dead code? Since overridden by `__eq__` defined on `entity_instance`.
     def __eq__(self, other: entity_instance_mixin) -> bool:
         if not isinstance(other, entity_instance_mixin):
             if not self.is_entity():

src/ifcwrap/IfcParseWrapper.i

@@ -38,6 +38,10 @@
 %ignore express::Select;
 %ignore express::DeclaredType;
 
+// operator< takes a reference, so SWIG's auto __lt__ crashes on None.
+// entity_instance_mixin already has a safe __lt__; let it inherit through.
+%ignore express::Base::operator<;
+
 %ignore in_memory_file_storage;
 %ignore rocks_db_file_storage;
 // Available as get_inverse().
@@ -389,6 +393,66 @@ private:
 	%}
 }
 
+%{
+// Shared by __eq__/__ne__ below - %extend methods can't call each other directly.
+// Initially was added as __eq__, but swig doesn't allow calling one extended method from another.
+static bool express_Base_equals(const express::Base* self, const express::Base* other) {
+	// This logic is not perfect and is not fully consistent with __hash__
+	if (!other) {
+		return false;
+	} else if (self->identity() == other->identity()) {
+		return true;
+	} else if (&self->declaration() != &other->declaration()) {
+		return false;
+	} else if ((self->file() != other->file()) || (self->declaration().as_entity() == nullptr)) {
+		// 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;
+	}
+}
+%}
+
 %extend express::Base {
 
 	// 0 = not found
@@ -508,58 +572,16 @@ private:
 		return $self->get_attribute_value((unsigned)i);
 	}
 
-	bool __eq__(const express::Base& other) const {
-		// This logic is not perfect and is not fully consistent with __hash__
-		if ($self->identity() == other.identity()) {
-			return true;
-		} else if (&$self->declaration() != &other.declaration()) {
-			return false;
-		} else if (($self->file() != other.file()) || ($self->declaration().as_entity() == nullptr)) {
-			// 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);
+	// `other` is a pointer, not a reference, so Python `None` converts to
+	// nullptr instead of SWIG raising a null-reference error before this
+	// body runs.
+	bool __eq__(const express::Base* other) const {
+		return express_Base_equals($self, other);
+	}
 
-				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;
-		}
+	// Override operator!= to support None or other data types in `other`.
+	bool __ne__(const express::Base* other) const {
+		return !express_Base_equals($self, other);
 	}
 
 	size_t __hash__() const {