@vmuriart what happens with multiple arguments, when overloaded methods have object argument?

@denfromufa good question. No idea.

foo(int a, object b)
foo(object a, int b)
foo(object a, object b)
foo(int a, int b)

These 4 functions look enough to cover the possibilities or do I need more?

we need one super-loaded method which covers all these cases from this meta-issue :)

#265

That sounds much broader than the scope of this pull_request.
I'm planning on refactoring methodbinder to clean up alot of what is going in there.

@vmuriart I understand, your example looks good. My suggestion was not serious anyway. But we may hit issues when dealing with sub-classes and ambiguous cases in the overload. IMO, pythonnet should be very strict by default in method resolution. See examples how C# compiler had to break method overloading in .NET 4.0:

http://csharpindepth.com/Articles/General/Overloading.aspx

@denfromufa No worries. My goal is to restore similar behavior to what pythonnet==2.0.0 had irt method objects. I had to freeze the version of pythonnet I was using to v2.0 because of it.
I'm ok w switching to a stricter method resolution, as long as it is part of v3.0 😄

@denfromufa Multiple arguments with objects works as expected.

@vmuriart no failure cases in overloads to make sure that they fail. No strings in overloaded method arguments when creating the methods in c#, but then you use strings as arguments when invoking from python.

I think I understood your request correctly. I added a test to check for failure/exception.

@denfromufa I think you are asking for these tests I added them on a separate commit.

Earlier when I was working on this pull request I used those tests to make sure I wasn't reverting the behavior you are describing. On earlier implementations, I reverted the behavior so those tests kept me in check.

The string-string check is analogous to the int-int check vs obj-obj check

@vmuriart string check is not analogous, you are assuming that strings are object in your tests. In fact string is subclass of object.

@denfromufa int are also a subclass of System.Object.

System.Object
   System.ValueType
      System.Int32

Sources:

• http://stackoverflow.com/a/8153167/5208670
• https://msdn.microsoft.com/en-us/library/system.string(v=vs.110).aspx

yes, int is also object subclass. However you use int in C# overloaded methods, while not using strings. Let me give some examples.

@vmuriart here is what I suggested to add. I cannot test locally, due to #385

public static string TestOverloadedObject(object o)
        {
            return "Got object";
        }

        public static string TestOverloadedObjectTwo(int a, int b)
        {
            return "Got int-int";
        }

        public static string TestOverloadedObjectTwo(string a, string b)
        {
            return "Got string-string";
        }

        public static string TestOverloadedObjectTwo(string a, int b)
        {
            return "Got string-int";
        }

        public static string TestOverloadedObjectTwo(string a, object b)
        {
            return "Got string-object";
        }

        public static string TestOverloadedObjectTwo(int a, object b)
        {
            return "Got int-object";
        }

        public static string TestOverloadedObjectTwo(object a, int b)
        {
            return "Got object-int";
        }

        public static string TestOverloadedObjectTwo(object a, object b)
        {
            return "Got object-object";
        }

        public static string TestOverloadedObjectThree(object a, int b)
        {
            return "Got object-int";
        }

        public static string TestOverloadedObjectThree(int a, object b)
        {
            return "Got int-object";
        }

def test_object_in_multiparam():
    """Test method with object multiparams behaves"""

    res = MethodTest.TestOverloadedObjectTwo(5, 5)
    assert res == "Got int-int"

    res = MethodTest.TestOverloadedObjectTwo(5, "foo")
    assert res == "Got int-object"

    res = MethodTest.TestOverloadedObjectTwo("foo", 5)
    assert res == "Got object-int"

    res = MethodTest.TestOverloadedObjectTwo("foo", "bar")
    assert res == "Got object-object"

    res = MethodTest.TestOverloadedObjectTwo("foo", System.Object)
    assert res == "Got string-object"
   
    res = MethodTest.TestOverloadedObjectTwo("foo", "bar")
    assert res == "Got string-string"

    res = MethodTest.TestOverloadedObjectTwo("foo", 5)
    assert res == "Got string-int"

@denfromufa Added your tests, but had to modify remove a couple of the conditions I had added earlier since the function signatures had changed.

@vmuriart I left 2 comments inline with the code

@denfromufa Added the parenthesis. Good to squash-merge?

you did not address the second comment. @vmuriart will this break if you add:

public static string TestOverloadedObjectTwo(int a, string b)
        {
            return "Got int-string";
        }

There was only a single comment.

And the answer is no. It simply changes the result of which method is used as it will pick the closest match it can find before it picked object.

so can you add it? because testing closest match is important.

so you don't see this?

It's not there. It even says on your screen shot that its pending.

Codecov Report

Merging #377 into master will increase coverage by 0.07%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #377      +/-   ##
==========================================
+ Coverage   63.27%   63.35%   +0.07%     
==========================================
  Files          61       61              
  Lines        5226     5226              
  Branches      860      860              
==========================================
+ Hits         3307     3311       +4     
+ Misses       1697     1695       -2     
+ Partials      222      220       -2

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e753b72...0ae9c38. Read the comment docs.

Added test and its passing as well.

@denfromufa test added and passing as well.

@vmuriart I'm still surprised that this does not break ;) i approve on my side. in the future we should definitely improve testing all these combinations of arguments by using hypothesis or parametrizing pytest. Let's release v2.3.0.dev1 version after this merge.

@filmor @tonyroberts please review!

@denfromufa I didn't want to introduce parametrizing tests yet. I figured I'd slowly introduce py.test features instead of all at once.