Basic operator overloads support by amos402 · Pull Request #907 · pythonnet/pythonnet

amos402 · 2019-07-05T17:04:37Z

What does this implement/fix? Explain your changes.

Support the operator for objects.

I found this code from my branch before, not only support the operator but also refactored the MethodBinder. @lostmsu I realized we just did the same thing. But a little different was I intended to support the method like foo(int x = 0, params object[] bar) can be call with foo(1,1,1), you can see the change from the pytest. I think we need to discuss how to merge them.
I develop it base on Python3, some test cases may fail on 2 now.

Does this close any currently open issues?

#906

Any other comments?

...

Checklist

Check all those that are applicable and complete.

Make sure to include one or more tests for your change
If an enhancement PR, please create docs and at best an example
Add yourself to AUTHORS
Updated the CHANGELOG

lostmsu

Completed initial pass with some comments.

src/runtime/methodbinder.cs

lostmsu · 2019-07-06T05:15:55Z

src/runtime/methodbinder.cs

+        {
+            if (tp == null)
+            {
+                yield break;


I think this should throw instead, and the caller should explicitly handle null case.

lostmsu · 2019-07-06T05:19:32Z

src/runtime/methodbinder.cs

+                MethodInfo method;
+                try
+                {
+                    method = t.MakeGenericMethod(tp);


You can yield return right here

lostmsu · 2019-07-06T05:19:47Z

src/runtime/methodbinder.cs

+                }
+                catch (ArgumentException)
+                {
+                    method = null;


You can continue right here

lostmsu · 2019-07-06T05:26:44Z

src/runtime/methodbinder.cs

+                    Type param = pi[n].ParameterType;
+
+                    if (!param.IsGenericParameter && !IsNullableOf(sig, param) &&
+                        !param.IsAssignableFrom(sig))


Can you explain what this method was supposed to be doing, and what your change enables it to do now? It is actually not clear what genericTp and sigTp represent from their names :(

lostmsu · 2019-07-06T05:50:48Z

src/runtime/operator.cs

+
+        public static void FixupSlots(IntPtr pyType, Type clrType)
+        {
+            IntPtr tp_as_number = Marshal.ReadIntPtr(pyType, TypeOffset.tp_as_number);


This does not seem to be used anywhere

😂opps, some methods are just used in my code, so it maybe not used here.

src/runtime/pythonengine.cs

lostmsu · 2019-07-06T05:54:12Z

src/runtime/typemanager.cs

+            {
+                throw new PythonException();
+            }
+            OperatorMethod.FixupSlots(type, clrType);


Should not the fixup happen before PyType_Ready?

It could be.

@amos402 From Python doc:

Finalize a type object. This should be called on all type objects to finish their initialization. This function is responsible for adding inherited slots from a type’s base class. Return 0 on success, or return -1 and sets an exception on error.

Unfortunately, the description does not clarify if the behavior could change in the future :/ I'd try to move FixupSlots up, and see if it works that way.

src/tests/test_array.py

lostmsu · 2019-07-06T05:57:08Z

src/tests/test_method.py

-    assert result[0] == 'one'
-    assert result[1] == 'two'
-    assert result[2] == 'three'
+    # Skip these temporally cause of the changes of array parameter calling


Is this change not ready?
.NET permits arrays to be passed in place of params object[] arguments. If it was allowed previously, but is not after your change, it removes a feature, which should not be done.

tminka · 2020-12-11T12:08:41Z

src/runtime/operator.cs

+            return OpMethodMap.ContainsKey(methodName);
+        }
+
+        public static bool IsPyOperatorMethod(string pyMethodName)


This method seems to be unused.

Looks like this was used later for delegates support? joonhwan@b154475

tminka · 2020-12-11T15:53:01Z

Fixes #1312

christabella · 2021-01-07T08:08:22Z

Could be closed in favour of the updated version at #1324

amos402 added 2 commits July 6, 2019 00:08

Basic operator overloads support

799bdae

Tests for foo(int x = 0, params object[] bar)

e483fc9

amos402 mentioned this pull request Jul 5, 2019

CLR operator and Python operator are not bound #906

Closed

lostmsu requested changes Jul 6, 2019

View reviewed changes

filmor added 2 commits August 2, 2019 12:26

Merge branch 'master' into operator

96ac36c

Merge branch 'master' into operator

8e9a8f5

tminka reviewed Dec 11, 2020

View reviewed changes

christabella mentioned this pull request Dec 16, 2020

Operator overloads support #1324

Merged

4 tasks

lostmsu closed this Jan 7, 2021

Conversation

amos402 commented Jul 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this implement/fix? Explain your changes.

Does this close any currently open issues?

Any other comments?

Checklist

Uh oh!

lostmsu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lostmsu Jul 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tminka commented Dec 11, 2020

Uh oh!

christabella commented Jan 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

amos402 commented Jul 5, 2019 •

edited

Loading

lostmsu Jul 6, 2019 •

edited

Loading