Why not have it asDictionary<int, MethodObject>, mapping directly from{Py_EQ, ...} to corresponding operator implementation?

Generally, you'd docls.richcompare.TryGetValue(op, out var methodObject) instead of having separateContainsKey check.

I don't think the code inside thisif should ever be skipped. How woulda > (1,2) work ifa was a .NET object with> operator? Because(1,2) is a tuple, they'd be passed toInvoke as two args instead of 1.

Consequently,free will alwasy betrue and not needed.

Probably a good idea to add a test. I believe you'd need anoperator >(SomeClass a, PyObject b) defined and to checkb indeed receives the tuple(1,2).

Indeed ifb were a tuple thisif would be skipped. Once removing theif, the operator method does receiveb asPyObject, but I get anAccessViolationException : Attempted to read or write protected memory. so I thinkb is wrongly parsed? I suppose a tuple when converted into a PyObject should probably not look like this:

I think that error message might have been a flaky bug, because now the variable states are the same but the error is:

Message:     Python.Runtime.PythonException : TypeError : '>=' not supported between instances of 'int' and 'tuple'

This is because to work withPyObject instances in C# callbacks you must hold GIL. E.g. the method body must be insideusing (Py.GIL()) { ... block. It applies to debugging too.

Thanks, I didn't know about that. I put theoperator >(Obj a, PyObject b) body in aPy.GIL block and can now see thatb has a value of{(1, 2)}

I added 2/6 of the tuple comparison tests but it's done in a bit of a strange way. In the test I'm assuming that the PyObject is a tuple; is that okay?

(Below is resolved)

However, the same error message persists, and when I try to debug

pythonnet/src/runtime/methodbinder.cs

Line 847 ind6c0081

result=binding.info.Invoke(binding.inst,BindingFlags.Default,null,binding.args,null);

I get this error

Maybe I'm missing something simple again, should the GIL block should also be wrapping that statement? But that would make the change bigger than I thought. Are tuples commonly used enough for comparison operators, or can I put the tuple test in a different PR?

I don't think you need to add a test with a tuple for each comparison operator. One is enough.

The failure on the screenshot is only happening because you have debugger attached. It tries to callToString on somePyObject to show in Watches window, but it does not work withoutPy.GIL

I think this is error-prone.op_LessThan does not really implementtp_richcompare. It would be better to have a separateComparisonOpMap,string -> string, and fixIsOperatorMethod correspondingly.

It would also remove the need to changeFixupSlots below.

Makes sense, thanks! Should thatComparisonOpMap also be merged with the other map forop_LessThan ->Py_LT?

I added thisOperatorMethod.ComparisonOpMap in the latest commit, without merging it with theClassBase.CilToPyOpMap, although they both have the same keys so perhaps they should be together inOperatorMethod.

Direct comment to the reason why we don't want to override, otherwise it does not explain anything and simply says what code in this method does (which is generally not very useful).

Something like "comparison operators are handled byClassBase.tp_richcompare"

unchecked(159832395 + Num.GetHashCode())

Please, remove unnecessaryif andfree boolean (alwaystrue)

NIT:!OpMethodMap .ContainsKey(method)

Of course, thanks!