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

Generally, you'd do cls.richcompare.TryGetValue(op, out var methodObject) instead of having separate ContainsKey check.

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

Consequently, free will alwasy be true and not needed.

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

Indeed if b were a tuple this if would be skipped. Once removing the if, the operator method does receive b as PyObject, but I get an AccessViolationException : Attempted to read or write protected memory. so I think b 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 with PyObject instances in C# callbacks you must hold GIL. E.g. the method body must be inside using (Py.GIL()) { ... block. It applies to debugging too.

Thanks, I didn't know about that. I put the operator >(Obj a, PyObject b) body in a Py.GIL block and can now see that b 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 in d6c0081

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 call ToString on some PyObject to show in Watches window, but it does not work without Py.GIL

I think this is error-prone. op_LessThan does not really implement tp_richcompare. It would be better to have a separate ComparisonOpMap, string -> string, and fix IsOperatorMethod correspondingly.

It would also remove the need to change FixupSlots below.

Makes sense, thanks! Should that ComparisonOpMap also be merged with the other map for op_LessThan -> Py_LT?

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

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 by ClassBase.tp_richcompare"

unchecked(159832395 + Num.GetHashCode())

Please, remove unnecessary if and free boolean (always true)

NIT: !OpMethodMap .ContainsKey(method)

Of course, thanks!