From e84731f8e8834d3eeac2fa442ccb4d6848a1f9c5 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 30 Jun 2022 15:24:00 -0700 Subject: [PATCH 1/3] implemented dynamic equality and inequality for PyObject instances fixed unhandled Python errors during comparison attempts fixes https://github.com/pythonnet/pythonnet/issues/1848 --- CHANGELOG.md | 2 + src/embed_tests/dynamic.cs | 22 +++++++++ src/runtime/PythonTypes/PyObject.cs | 75 +++++++++++++++++++++++------ src/runtime/Runtime.cs | 25 ---------- 4 files changed, 83 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52ee08484..9b5dd1816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ details about the cause of the failure able to access members that are part of the implementation class, but not the interface. Use the new `__implementation__` or `__raw_implementation__` properties to if you need to "downcast" to the implementation class. +- BREAKING: `==` and `!=` operators on `PyObject` instances now use Python comparison + (previously was equivalent to `object.ReferenceEquals(,)`) - BREAKING: Parameters marked with `ParameterAttributes.Out` are no longer returned in addition to the regular method return value (unless they are passed with `ref` or `out` keyword). - BREAKING: Drop support for the long-deprecated CLR.* prefix. diff --git a/src/embed_tests/dynamic.cs b/src/embed_tests/dynamic.cs index 0a181231c..6e3bfc4cb 100644 --- a/src/embed_tests/dynamic.cs +++ b/src/embed_tests/dynamic.cs @@ -128,6 +128,28 @@ public void PassPyObjectInNet() Assert.IsTrue(sys.testattr1.Equals(sys.testattr2)); } + // regression test for https://github.com/pythonnet/pythonnet/issues/1848 + [Test] + public void EnumEquality() + { + using var scope = Py.CreateScope(); + scope.Exec(@" +import enum + +class MyEnum(enum.IntEnum): + OK = 1 + ERROR = 2 + +def get_status(): + return MyEnum.OK +" +); + + dynamic MyEnum = scope.Get("MyEnum"); + dynamic status = scope.Get("get_status").Invoke(); + Assert.IsTrue(status == MyEnum.OK); + } + // regression test for https://github.com/pythonnet/pythonnet/issues/1680 [Test] public void ForEach() diff --git a/src/runtime/PythonTypes/PyObject.cs b/src/runtime/PythonTypes/PyObject.cs index cfd3e7158..3d48e22ed 100644 --- a/src/runtime/PythonTypes/PyObject.cs +++ b/src/runtime/PythonTypes/PyObject.cs @@ -1075,12 +1075,9 @@ public virtual bool Equals(PyObject? other) { return true; } - int r = Runtime.PyObject_Compare(this, other); - if (Exceptions.ErrorOccurred()) - { - throw PythonException.ThrowLastAsClrException(); - } - return r == 0; + int result = Runtime.PyObject_RichCompareBool(obj, other.obj, Runtime.Py_EQ); + if (result < 0) throw PythonException.ThrowLastAsClrException(); + return result != 0; } @@ -1304,6 +1301,18 @@ public override bool TryConvert(ConvertBinder binder, out object? result) return false; } + private bool TryCompare(PyObject arg, int op, out object @out) + { + int result = Runtime.PyObject_RichCompareBool(this.obj, arg.obj, op); + @out = result != 0; + if (result < 0) + { + Exceptions.Clear(); + return false; + } + return true; + } + public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg, out object? result) { using var _ = Py.GIL(); @@ -1352,11 +1361,9 @@ public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg res = Runtime.PyNumber_InPlaceXor(this.obj, ((PyObject)arg).obj); break; case ExpressionType.GreaterThan: - result = Runtime.PyObject_Compare(this.obj, ((PyObject)arg).obj) > 0; - return true; + return this.TryCompare((PyObject)arg, Runtime.Py_GT, out result); case ExpressionType.GreaterThanOrEqual: - result = Runtime.PyObject_Compare(this.obj, ((PyObject)arg).obj) >= 0; - return true; + return this.TryCompare((PyObject)arg, Runtime.Py_GE, out result); case ExpressionType.LeftShift: res = Runtime.PyNumber_Lshift(this.obj, ((PyObject)arg).obj); break; @@ -1364,11 +1371,9 @@ public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg res = Runtime.PyNumber_InPlaceLshift(this.obj, ((PyObject)arg).obj); break; case ExpressionType.LessThan: - result = Runtime.PyObject_Compare(this.obj, ((PyObject)arg).obj) < 0; - return true; + return this.TryCompare((PyObject)arg, Runtime.Py_LT, out result); case ExpressionType.LessThanOrEqual: - result = Runtime.PyObject_Compare(this.obj, ((PyObject)arg).obj) <= 0; - return true; + return this.TryCompare((PyObject)arg, Runtime.Py_LE, out result); case ExpressionType.Modulo: res = Runtime.PyNumber_Remainder(this.obj, ((PyObject)arg).obj); break; @@ -1376,8 +1381,9 @@ public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg res = Runtime.PyNumber_InPlaceRemainder(this.obj, ((PyObject)arg).obj); break; case ExpressionType.NotEqual: - result = Runtime.PyObject_Compare(this.obj, ((PyObject)arg).obj) != 0; - return true; + return this.TryCompare((PyObject)arg, Runtime.Py_NE, out result); + case ExpressionType.Equal: + return this.TryCompare((PyObject)arg, Runtime.Py_EQ, out result); case ExpressionType.Or: res = Runtime.PyNumber_Or(this.obj, ((PyObject)arg).obj); break; @@ -1402,6 +1408,40 @@ public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg return true; } + public static bool operator ==(PyObject? a, PyObject? b) + { + if (a is null && b is null) + { + return true; + } + if (a is null || b is null) + { + return false; + } + + using var _ = Py.GIL(); + int result = Runtime.PyObject_RichCompareBool(a.obj, b.obj, Runtime.Py_EQ); + if (result < 0) throw PythonException.ThrowLastAsClrException(); + return result != 0; + } + + public static bool operator !=(PyObject? a, PyObject? b) + { + if (a is null && b is null) + { + return false; + } + if (a is null || b is null) + { + return true; + } + + using var _ = Py.GIL(); + int result = Runtime.PyObject_RichCompareBool(a.obj, b.obj, Runtime.Py_NE); + if (result < 0) throw PythonException.ThrowLastAsClrException(); + return result != 0; + } + // Workaround for https://bugzilla.xamarin.com/show_bug.cgi?id=41509 // See https://github.com/pythonnet/pythonnet/pull/219 internal static object? CheckNone(PyObject pyObj) @@ -1436,14 +1476,17 @@ public override bool TryUnaryOperation(UnaryOperationBinder binder, out object? case ExpressionType.Not: r = Runtime.PyObject_Not(this.obj); result = r == 1; + if (r == -1) Exceptions.Clear(); return r != -1; case ExpressionType.IsFalse: r = Runtime.PyObject_IsTrue(this.obj); result = r == 0; + if (r == -1) Exceptions.Clear(); return r != -1; case ExpressionType.IsTrue: r = Runtime.PyObject_IsTrue(this.obj); result = r == 1; + if (r == -1) Exceptions.Clear(); return r != -1; case ExpressionType.Decrement: case ExpressionType.Increment: diff --git a/src/runtime/Runtime.cs b/src/runtime/Runtime.cs index 6ad1d459f..1eeb96b54 100644 --- a/src/runtime/Runtime.cs +++ b/src/runtime/Runtime.cs @@ -962,31 +962,6 @@ internal static IntPtr PyObject_CallObject(IntPtr pointer, IntPtr args) internal static int PyObject_RichCompareBool(BorrowedReference value1, BorrowedReference value2, int opid) => Delegates.PyObject_RichCompareBool(value1, value2, opid); - internal static int PyObject_Compare(BorrowedReference value1, BorrowedReference value2) - { - int res; - res = PyObject_RichCompareBool(value1, value2, Py_LT); - if (-1 == res) - return -1; - else if (1 == res) - return -1; - - res = PyObject_RichCompareBool(value1, value2, Py_EQ); - if (-1 == res) - return -1; - else if (1 == res) - return 0; - - res = PyObject_RichCompareBool(value1, value2, Py_GT); - if (-1 == res) - return -1; - else if (1 == res) - return 1; - - Exceptions.SetError(Exceptions.SystemError, "Error comparing objects"); - return -1; - } - internal static int PyObject_IsInstance(BorrowedReference ob, BorrowedReference type) => Delegates.PyObject_IsInstance(ob, type); From 2edd8000e49caa24b6a7feb3f192eac0fd09161b Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Tue, 28 Jun 2022 14:45:43 +0200 Subject: [PATCH 2/3] Fix broken prefix and debug leftover Additionally, fixes a type hint and makes sure that the new default behaviour is to use the environment variable if given. --- pythonnet/__init__.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pythonnet/__init__.py b/pythonnet/__init__.py index fa6ed45cf..9876a0bec 100644 --- a/pythonnet/__init__.py +++ b/pythonnet/__init__.py @@ -3,7 +3,7 @@ from typing import Dict, Optional, Union import clr_loader -__all__ = ["set_runtime", "set_default_runtime", "load"] +__all__ = ["set_runtime", "set_runtime_from_env", "load"] _RUNTIME: Optional[clr_loader.Runtime] = None _LOADER_ASSEMBLY: Optional[clr_loader.wrappers.Assembly] = None @@ -30,7 +30,7 @@ def set_runtime(runtime: Union[clr_loader.Runtime, str], **params: str) -> None: def _get_params_from_env(prefix: str) -> Dict[str, str]: from os import environ - full_prefix = f"PYTHONNET_{prefix.upper()}" + full_prefix = f"PYTHONNET_{prefix.upper()}_" len_ = len(full_prefix) env_vars = { @@ -63,8 +63,8 @@ def _create_runtime_from_spec( raise RuntimeError(f"Invalid runtime name: '{spec}'") -def set_default_runtime() -> None: - """Set up the default runtime +def set_runtime_from_env() -> None: + """Set up the runtime using the environment This will use the environment variable PYTHONNET_RUNTIME to decide the runtime to use, which may be one of netfx, coreclr or mono. The parameters @@ -80,16 +80,13 @@ def set_default_runtime() -> None: """ from os import environ - print("Set default RUNTIME") - raise RuntimeError("Shouldn't be called here") - spec = environ.get("PYTHONNET_RUNTIME", "default") runtime = _create_runtime_from_spec(spec) set_runtime(runtime) def load( - runtime: Union[clr_loader.Runtime, str] = "default", **params: Dict[str, str] + runtime: Union[clr_loader.Runtime, str, None] = None, **params: str ) -> None: """Load Python.NET in the specified runtime @@ -102,7 +99,10 @@ def load( return if _RUNTIME is None: - set_runtime(runtime, **params) + if runtime is None: + set_runtime_from_env() + else: + set_runtime(runtime, **params) if _RUNTIME is None: raise RuntimeError("No valid runtime selected") From 4b6ac1246362a6ecccbaf54bcdd15b105c11ea5d Mon Sep 17 00:00:00 2001 From: Benedikt Reinartz Date: Tue, 5 Jul 2022 23:10:42 +0200 Subject: [PATCH 3/3] Bump release candidate version --- version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.txt b/version.txt index cf8213193..d15be84db 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -3.0.0-rc1 +3.0.0-rc2